I wrote:
> While fooling around in the planner I realized that I have no idea what
> outer-level aggregates mean in a LATERAL subquery, and neither does
> Postgres:
> regression=# select 1 from tenk1 a, lateral (select * from int4_tbl b where f1 = max(a.unique1)) x;
> ERROR: plan should not reference subplan's variable
> I don't see anything prohibiting this in SQL:2008, but ordinarily this
> would be taken to be an outer-level aggregate, and surely that is not
> sensible in the LATERAL subquery. For the moment it seems like a good
> idea to disallow it, though I am not sure where is a convenient place
> to test for such things. Has anyone got a clue about whether this is
> well-defined, or is it simply an oversight in the spec?
On further reflection I think this is indeed disallowed by spec. The
outer query is clearly the "aggregation query" of the aggregate, and the
aggregate appears inside that query's FROM list, therefore it's no good;
see SQL:2008 6.9 <set function specification> syntax rules 6 and 7.
(I missed this before because it's not under the aggregate function
heading.)
So the problem here is just that parseCheckAggregates neglects to grovel
through subqueries-in-FROM looking for aggregates of the current level.
Since AFAICS the case cannot arise without LATERAL, this isn't really a
pre-existing bug.
I find it fairly annoying though that parseCheckAggregates (and likewise
parseCheckWindowFuncs) have to dig through previously parsed query trees
to look for misplaced aggregates; so adding even more of that is grating
on me. It would be a lot cleaner if transformAggregateCall and
transformWindowFuncCall could throw these errors immediately. The
reason they can't is lack of context about what portion of the query we
are currently parsing. I'm thinking it'd be worthwhile to add an enum
field to ParseState that shows whether we're currently parsing the
associated query level's target list, WHERE clause, GROUP BY clause,
etc. The easiest way to ensure this gets set for all cases should be to
add the enum value as another argument to transformExpr(), which
would then save it into the ParseState for access by subsidiary
expression transformation functions.
Thoughts?
regards, tom lane