Re: Wrong results with grouping sets

On Fri, May 24, 2024 at 9:08 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> On the basis of the parser infrastructure fixup, 0002 patch adds the
> nullingrel bit that references the grouping RTE to the grouping
> expressions.

I found a bug in the v6 patch. The following query would trigger the
Assert in make_restrictinfo that the given subexpression should not be
an AND clause.

select max(a) from t group by a > b and a = b having a > b and a = b;

This is because the expression 'a > b and a = b' in the HAVING clause is
replaced by a Var that references the GROUP RTE. When we preprocess the
columns of the GROUP RTE, we do not know whether the grouped expression
is a havingQual or not, so we do not perform make_ands_implicit for it.
As a result, after we replace the group Var in the HAVING clause with
the underlying grouping expression, we will have a havingQual that is an
AND clause.

As we know, in the planner we need to first preprocess all the columns
of the GROUP RTE. We also need to replace any Vars in the targetlist
and HAVING clause that reference the GROUP RTE with the underlying
grouping expressions. To fix the mentioned issue, I choose the perform
this replacement before we preprocess the targetlist and havingQual, so
that the make_ands_implicit would be performed when we preprocess the
havingQual.

One problem with this is, when we preprocess the targetlist and
havingQual, we would see already-planned tree, which is generated by the
preprocessing work for the grouping expressions and then substituted for
the GROUP Vars in the targetlist and havingQual. This would break the
Assert 'Assert(!IsA(node, SubPlan))' in flatten_join_alias_vars_mutator
and process_sublinks_mutator. I think we can just return the
already-planned tree unchanged when we see it in the preprocessing
process.

Hence here is the v7 patchset. I've also added detailed commit messages
for the two patches.

Thanks
Richard