Re: [PATCH] Fix grouping-set nulling of variable-free grouping expressions

From: Haibo Yan <tristan(dot)yim(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, shihao zhong <zhong950419(at)gmail(dot)com>
Subject: Re: [PATCH] Fix grouping-set nulling of variable-free grouping expressions
Date: 2026-10-10 17:03:48
Message-ID: CABXr29EQMOv1nhyLpkR_kPKcJJn4U29aC-NwDP5=tRRtMjbFxQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 9, 2026 at 11:10 PM Haibo Yan <tristan(dot)yim(at)gmail(dot)com> wrote:
>
> Hi Hackers,
>
> I found a wrong-result issue with grouping sets when a grouping
> expression is reduced to a constant during planning.
>
> For example:
>
> CREATE TABLE t (a int NOT NULL);
> INSERT INTO t VALUES (1);
>
> SELECT CASE WHEN a IS NOT NULL THEN 'yes' ELSE 'no' END,
> GROUPING(a IS NOT NULL), count(*)
> FROM t
> GROUP BY ROLLUP (a IS NOT NULL)
> ORDER BY 2;
>
> PostgreSQL 18 returns:
>
> yes | 0 | 1
> no | 1 | 1
>
> PostgreSQL 19 instead returns:
>
> yes | 0 | 1
> yes | 1 | 1
>
> The grand-total row should contain 'no', since the grouping expression
> is NULL in that grouping set.
>
> This regression was exposed by commit e2debb64380, which allows
> "a IS NOT NULL" to be folded to true based on the column's NOT NULL
> constraint. The folding itself is valid for the grouping input.
>
> The underlying issue predates this change. For example:
>
> SELECT (1 + 1) * 10
> FROM generate_series(1, 10)
> GROUP BY ROLLUP (1 + 1);
>
> PostgreSQL 18 already returns 20 for the grand-total row, where the
> result should be NULL.
>
> The problem is that flatten_group_exprs() substitutes the preprocessed
> grouping expression for an RTE_GROUP reference. For variable-free
> expressions, mark_nullable_by_grouping() wraps the expression in a
> PlaceHolderVar, but creates a new PHV ID for each reference to the
> same grouping output.
>
> Consequently, setrefs may fail to match an upper reference to the
> actual grouping column. The expression is then evaluated separately,
> bypassing the NULL inserted for the corresponding grouping set.
>
> The attached patch caches one PHV ID per RTE_GROUP attribute in
> PlannerInfo, so references to the same grouping output share an ID.
> Different grouping attributes retain distinct IDs, even when their
> expressions are folded to the same constant.
>
> This fixes both the PostgreSQL 19 regression and the pre-existing
> variable-free grouping-expression issue, without restricting constant
> folding or changing the aggregate executor.
>
> The patch has been tested on master and REL_19_STABLE with assertions
> enabled. Core regression, isolation, and relevant TAP tests pass.
> Regression tests cover ROLLUP, CUBE, GROUPING SETS, HAVING, LATERAL,
> and distinct grouping expressions folded to identical constants.
>
> Given the wrong-result regression in PostgreSQL 19, I'd appreciate
> review of whether this fix is appropriate for REL_19_STABLE.
>
> The underlying issue also affects PostgreSQL 18. Backpatching there
> would require some branch-specific adaptation and testing.
>
> Comments and review are welcome.
>
> Thanks,
> Haibo

Hi,

Attached is v2, updated to current master.

There are no changes to the implementation or regression tests
from v1. The patch applies cleanly to both master and
REL_19_STABLE.

I also added the following contributor credit:

Reported-by: Shihao Zhong <zhong950419(at)gmail(dot)com>

The patch has been independently tested on both branches with
assertions enabled. Core regression (239/239), isolation
(134/134), and the relevant postgres_fdw tests all pass.

Thanks,
Haibo

Attachment Content-Type Size
0002-Fix-preexisting-grouping-expression-bug.patch application/octet-stream 8.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2026-10-10 17:15:27 Re: Policy for Abandoned Extensions
Previous Message Fujii Masao 2026-10-10 17:02:14 Re: REPACK: warn about skipping foreign partitions