Horton

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

Richard Guo <guofenglinux@gmail.com>

View thread
From: Richard Guo <guofenglinux@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>, 邱宇航 <iamqyh@gmail.com>, Bruce Momjian <bruce@momjian.us>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2025-09-24T08:18:35Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix test case from 40c242830

  2. Fix pushdown of degenerate HAVING clauses

  3. Allow pushdown of HAVING clauses with grouping sets

  4. Mark expressions nullable by grouping sets

Attachments

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