I have experimented with this approach, and here is the outcome. The patch fixes Geoff's query, but it's still somewhat messy as I'm not experienced enough in the parser code. And the patch has not yet implemented the nullingrel bit manipulation trick. Before proceeding further with this patch, I'd like to know if it is going in the right direction.
I've spent some more time on this patch, and now it passes all the regression tests. But I had to hack explain.c and ruleutils.c to make the varprefix stuff work as it did before, which is not great.
One thing I'm not sure about is whether we need to also replace subexpressions in the arguments of GroupingFunc nodes with Vars referencing the new GROUP RTE. These arguments would not be executed at runtime, so it seems that we can just replace them. I tried to do that and found several plan changes in the regression tests. Such as
explain (verbose, costs off) select grouping(ss.x) from int8_tbl i1 cross join lateral (select (select i1.q1) as x) ss group by ss.x; QUERY PLAN ------------------------------------------------ GroupAggregate Output: GROUPING((SubPlan 1)), ((SubPlan 2)) Group Key: ((SubPlan 2)) -> Sort Output: ((SubPlan 2)), i1.q1 Sort Key: ((SubPlan 2)) -> Seq Scan on public.int8_tbl i1 Output: (SubPlan 2), i1.q1 SubPlan 2 -> Result Output: i1.q1 (11 rows)
If we substitute the subquery expression in the argument of GroupingFunc with the GROUP RTE's Var, the final plan would contain only one SubPlan instead of two.
Also the patch has not yet manipulated the nullingrel stuff. Maybe that can be done with the code in my v2 patch. But I think we'd better get the parser fixed first before stepping into that.
Also please ignore the comment and code format things in the patch as I haven't worked on them.
