Re: Wrong results with grouping sets

On Thu, May 16, 2024 at 5:43 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> 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.

Thanks
Richard