Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master

Hi Richard
> I think the following change fixes this problem.
>
> foreach(l, (List *) parse->havingQual)
> {
> Node *havingclause = (Node *) lfirst(l);
> + Relids having_relids;
>
> if (contain_agg_clause(havingclause) ||
> contain_volatile_functions(havingclause) ||
> contain_subplans(havingclause) ||
> (parse->groupClause && parse->groupingSets &&
> - bms_is_member(root->group_rtindex, pull_varnos(root,
> havingclause))))
> + ((having_relids = pull_varnos(root, havingclause)) == NULL ||
> + bms_is_member(root->group_rtindex, having_relids))))
> {
> /* keep it in HAVING */
> newHaving = lappend(newHaving, havingclause);
>
> This change essentially prevents variable-free HAVING clauses from
> being pushed down when there are any nonempty grouping sets. Of
> course, this could be done more precisely -- for example, we could
> restrict the prevention only to cases where empty grouping sets are
> also present, but I'm not sure it's worth the effort.
I think it makes sense. However, it is now too close to the release date of
v18.Awaiting Tom's feedback？

Thanks

On Wed, Sep 24, 2025 at 4:18 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> On Wed, Sep 24, 2025 at 12:10 PM Richard Guo <guofenglinux(at)gmail(dot)com>
> wrote:
> > In summary, the problem occurs when both of the following conditions
> > are met: 1) there are both nonempty and empty grouping sets, 2) there
> > are variable-free HAVING clauses.
> >
> > I think the following change fixes this problem.
> >
> > foreach(l, (List *) parse->havingQual)
> > {
> > Node *havingclause = (Node *) lfirst(l);
> > + Relids having_relids;
> >
> > if (contain_agg_clause(havingclause) ||
> > contain_volatile_functions(havingclause) ||
> > contain_subplans(havingclause) ||
> > (parse->groupClause && parse->groupingSets &&
> > - bms_is_member(root->group_rtindex, pull_varnos(root,
> > havingclause))))
> > + ((having_relids = pull_varnos(root, havingclause)) == NULL
> ||
> > + bms_is_member(root->group_rtindex, having_relids))))
> > {
> > /* keep it in HAVING */
> > newHaving = lappend(newHaving, havingclause);
> >
> > This change essentially prevents variable-free HAVING clauses from
> > being pushed down when there are any nonempty grouping sets. Of
> > course, this could be done more precisely -- for example, we could
> > restrict the prevention only to cases where empty grouping sets are
> > also present, but I'm not sure it's worth the effort.
>
> Here is the patch.
>
> - Richard
>