Restore nodeAgg.c's ability to check for improperly-nested aggregates.
authorTom Lane
Sun, 15 Oct 2017 23:19:19 +0000 (19:19 -0400)
committerTom Lane
Sun, 15 Oct 2017 23:19:19 +0000 (19:19 -0400)
commitcb591fcbfbba1df6fda1839ece53665e85e491e3
tree2ecf0d6170dc914cb4dd82e662694251ad0c00bc
parenta3b1c221893f739950e9232b4b789750f247cee5
Restore nodeAgg.c's ability to check for improperly-nested aggregates.

While poking around in the aggregate logic, I noticed that commit
8ed3f11bb 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 24a1e20f1.

In passing improve a few comments.

Back-patch to v10, just in case.
src/backend/executor/nodeAgg.c