| From: | Haibo Yan <tristan(dot)yim(at)gmail(dot)com> |
|---|---|
| To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
| Cc: | Melanie Plageman <melanieplageman(at)gmail(dot)com>, shihao zhong <zhong950419(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Subject: | Re: [PG19] Wrong results from NOT NULL-based expression simplification |
| Date: | 2026-10-10 15:27:05 |
| Message-ID: | CABXr29HeEns=cw290-Kfp3tYRa+oYYGVP51_cnSqPBUN20LrTw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Oct 9, 2026 at 5:53 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
>
> On Thu, Oct 8, 2026 at 12:41 AM Melanie Plageman
> <melanieplageman(at)gmail(dot)com> wrote:
> > On Mon, Oct 5, 2026 at 11:28 PM shihao zhong <zhong950419(at)gmail(dot)com> wrote:
> > > I used Opus to analyze the new features in PG19, and it found five
> > > queries that work on 18 and break on 19 and master. Each item has a
> > > script and a patch with its number.
>
> > Seems like there are a number of issues in this thread but so far I
> > see only one open item for the whole thing. Should there be more than
> > one open item?
>
> I've pushed fixes for #1, #3 and #4, after makeing cosmetic tweaks.
>
> Regarding #1, nothing more to add.
>
> Regarding #2, this is a bug in v17 and later, not a v19 regression, so
> I don't think it should be a v19 open item. And it's in the MERGE
> code, so I think Dean is the right person to look at it, not me.
>
> Regarding #3, it is not limited to COALESCE. CASE and NULLIF have the
> same problem when they are reduced to one of their inputs, and those
> date back to older branches. Even for COALESCE, the first commit that
> has this problem is 931766aae, not the NOT NULL-based simplification.
> The fix covers all three, but I only back-patched it to v19, since
> there are no field reports and it changes the typmod exposed by a
> reduced CASE expression.
>
> Regarding #4, the fix is to not trust NOT NULL on a virtual generated
> column of a partitioned table. But I don't like this fix at all. I
> think the real problem is that a partition is allowed to have a
> different virtual generation expression than its parent. The
> constraint is enforced with the partition's expression, while a query
> on the parent computes the column with the parent's expression. So
> even on v18, a NOT NULL column can read as NULL through the parent,
> and the same goes for CHECK constraints. That seems fragile to me,
> and I suspect it will bite us again. CC'ing Peter here.
>
> Regarding #5, it is not specific to v19 either. You get the same
> wrong result with CASE WHEN true THEN a END in place of COALESCE(a, 0),
> which has nothing to do with the new simplification. So I don't think
> it should be a v19 open item.
>
> - Richard
Hi Shihao, Richard,
Thanks, Shihao, for bringing the grouping-set issue to my attention.
I investigated a related problem and posted a separate patch in [1].
I also tested it against Shihao's 0005 patch.
The two patches address different identity problems.
My patch fixes the case where multiple references to the same
variable-free grouping expression receive different PHV IDs.
It reuses one PHV ID per RTE_GROUP attribute.
Patch 0005 addresses a different case, where distinct grouping
expressions simplify to the same expression containing Vars, causing
their references to become indistinguishable.
Neither patch fixes the other's reproducer. Both can be applied
together without conflicts in the C code.
However, I found a potential wrong-result regression in 0005
involving volatile expressions:
CREATE TABLE t (id int PRIMARY KEY, a int NOT NULL);
CREATE TABLE s (x int);
INSERT INTO t VALUES (1, 1), (2, 2), (3, 3);
INSERT INTO s VALUES (10), (20);
SELECT count(*)
FROM (
SELECT 1
FROM t JOIN s ON true
GROUP BY GROUPING SETS (
(t.a + random()),
(coalesce(t.a, 0) + random())
)
) ss;
Without 0005, this returned 12 rows, as expected.
With 0005, it returned 9.
The PHV introduced by 0005 appears to cause the volatile expression
to be evaluated at the scan of t rather than once per joined row.
Its value is then reused across multiple join output rows.
I also found that 0005 does not resolve some duplicate grouping
expressions involving argument-free SRFs.
These seem worth investigating separately. I suggest keeping
the two fixes independent for now.
Thanks,
Haibo
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Geoghegan | 2026-10-10 15:27:09 | Re: Hash index bucket split bug |
| Previous Message | Haibo Yan | 2026-10-10 14:38:34 | Re: addFkRecurseReferencing use unassigned fkconstraint->fk_with_period value |