Re: BUG #5025: Aggregate function with subquery in 8.3 and 8.4.

I wrote:
> I think the problem is probably that we need a PlaceHolderVar wrapper
> around the ROW() constructor.

I looked into this a bit more. The issue is that flattening of
subqueries that are inside an outer join first puts PlaceHolderVars
around non-nullable output expressions of the subquery, and then applies
ResolveNew() to substitute those expressions for upper references to the
subquery outputs. However, a whole-row Var referencing the subquery is
expanded by ResolveNew into a ROW() expression over the subquery
outputs. To preserve compatibility with pre-8.4 behavior, what we
really need here is to have a PlaceHolderVar around the ROW(), not on
the individual expressions inside it.

While it's not that hard to put in the PlaceHolderVar, the easy way to
do it would require passing the PlannerInfo "root" struct to ResolveNew,
which goes well beyond my threshold of pain from a modularity
standpoint --- ResolveNew isn't part of the planner and has no business
using that struct. ResolveNew's API is a study in ugliness already,
so I'm thinking it's time to bite the bullet and refactor it. The
idea that comes to mind is to provide a callback function and "void *"
context argument, which ResolveNew would call upon finding a Var that
needs substitution. The existing guts of ResolveNew would move into a
callback that is specific to rewriteHandler.c's uses, and we'd write a
different one for the planner. The PlannerInfo link would be hidden
within the "void *" argument so we'd avoid exposing it to rewriter code.

Comments?

I believe BTW that there are related issues in other places where we
expand composites into RowExprs. But the other places have been doing
that for awhile. I think that for 8.4 our goals should be limited to
not changing the behavior compared to prior releases. If any consensus
is reached on the general issue of how we want "row() is null" to
behave, then it'll be time to reconsider the other stuff.

regards, tom lane