Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master - Mailing list pgsql-hackers

From wenhui qiu
Subject Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master
Date
Msg-id CAGjGUAKo58TwHXXnp-LerP1vqCzetRdtThTK2Z=-yi+CShtWDQ@mail.gmail.com
Whole thread Raw
In response to Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master  (Richard Guo <guofenglinux@gmail.com>)
List pgsql-hackers
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@gmail.com> wrote:
On Wed, Sep 24, 2025 at 12:10 PM Richard Guo <guofenglinux@gmail.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

pgsql-hackers by date:

Previous
From: Chao Li
Date:
Subject: Re: GB18030-2022 Support in PostgreSQL
Next
From: John Naylor
Date:
Subject: Re: use radix tree for bitmap heap scan