• Tom Lane's avatar
    Restore nodeAgg.c's ability to check for improperly-nested aggregates. · 5fc438fb
    Tom Lane authored
    While poking around in the aggregate logic, I noticed that commit
    8ed3f11b broke the logic in nodeAgg.c that purports to detect nested
    aggregates, by moving initialization of regular aggregate argument
    expressions out of the code segment that checks for that.
    
    You could argue that this check is unnecessary, but it's not much code
    so I'm inclined to keep it as a backstop against parser and planner
    bugs.  However, there's certainly zero value in checking only some of
    the subexpressions.
    
    We can make the check complete again, and as a bonus make it a good
    deal more bulletproof against future mistakes of the same ilk, by
    moving it out to the outermost level of ExecInitAgg.  This means we
    need to check only once per Agg node not once per aggregate, which
    also seems like a good thing --- if the check does find something
    wrong, it's not urgent that we report it before the plan node
    initialization finishes.
    
    Since this requires remembering the original length of the aggs list,
    I deleted a long-obsolete stanza that changed numaggs from 0 to 1.
    That's so old it predates our decision that palloc(0) is a valid
    operation, in (digs...) 2004, see commit 24a1e20f.
    
    In passing improve a few comments.
    
    Back-patch to v10, just in case.
    5fc438fb
nodeAgg.c 131 KB