public inbox for [email protected]help / color / mirror / Atom feed
Re: BUG #19412: Wrong query result with not null constraint 8+ messages / 4 participants [nested] [flat]
* Re: BUG #19412: Wrong query result with not null constraint @ 2026-02-17 22:53 David Rowley <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: David Rowley @ 2026-02-17 22:53 UTC (permalink / raw) To: [email protected]; [email protected]; Tom Lane <[email protected]> On Wed, 18 Feb 2026 at 00:31, PG Bug reporting form <[email protected]> wrote: > create table a (id int, x_id int, y_id int); > insert into a values (1, 1, 1), (1, 2, 2), (1, 3, 3); > create table x (id int, nm text, constraint pk_x_id primary key (id)); > insert into x values (1, 'x1'), (2, 'x2'), (3, 'x3'); > create table y (id int, nm text, constraint pk_y_id primary key (id)); > insert into y values (1, 'y1'), (3, 'y3'), (4, 'y4'); > > select a.id, z.id > from a > join x on x.id = a.x_id > left join y on y.id = a.y_id > join lateral(select x.id > union all > select y.id) z on z.id is not null; Thanks for the reproducer. I'd say that y.id Var in the lateral join should be getting marked as nullable by the left join, but it's not being marked as nullable by anything. Tom, do you have any thoughts on the empty varnullingrels here? David ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #19412: Wrong query result with not null constraint @ 2026-02-18 01:51 Richard Guo <[email protected]> parent: David Rowley <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Richard Guo @ 2026-02-18 01:51 UTC (permalink / raw) To: David Rowley <[email protected]>; +Cc: [email protected]; [email protected]; Tom Lane <[email protected]> On Wed, Feb 18, 2026 at 7:54 AM David Rowley <[email protected]> wrote: > On Wed, 18 Feb 2026 at 00:31, PG Bug reporting form > <[email protected]> wrote: > > create table a (id int, x_id int, y_id int); > > insert into a values (1, 1, 1), (1, 2, 2), (1, 3, 3); > > create table x (id int, nm text, constraint pk_x_id primary key (id)); > > insert into x values (1, 'x1'), (2, 'x2'), (3, 'x3'); > > create table y (id int, nm text, constraint pk_y_id primary key (id)); > > insert into y values (1, 'y1'), (3, 'y3'), (4, 'y4'); > > > > select a.id, z.id > > from a > > join x on x.id = a.x_id > > left join y on y.id = a.y_id > > join lateral(select x.id > > union all > > select y.id) z on z.id is not null; > Thanks for the reproducer. > > I'd say that y.id Var in the lateral join should be getting marked as > nullable by the left join, but it's not being marked as nullable by > anything. Exactly. I think this is because when adjust_appendrel_attrs_mutator propagates the nullingrel bits from the parent rel's Var into the translated Var, it loses the translated Var's original bits. Instead of overwriting the translated Var's nullingrels, I think we should merge them. --- a/src/backend/optimizer/util/appendinfo.c +++ b/src/backend/optimizer/util/appendinfo.c @@ -291,8 +291,11 @@ adjust_appendrel_attrs_mutator(Node *node, var->varattno, get_rel_name(appinfo->parent_reloid)); if (IsA(newnode, Var)) { - ((Var *) newnode)->varreturningtype = var->varreturningtype; - ((Var *) newnode)->varnullingrels = var->varnullingrels; + Var *newvar = (Var *) newnode; + + newvar->varreturningtype = var->varreturningtype; + newvar->varnullingrels = bms_add_members(newvar->varnullingrels, + var->varnullingrels); } - Richard ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #19412: Wrong query result with not null constraint @ 2026-02-18 09:50 Richard Guo <[email protected]> parent: Richard Guo <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Richard Guo @ 2026-02-18 09:50 UTC (permalink / raw) To: David Rowley <[email protected]>; +Cc: [email protected]; [email protected]; Tom Lane <[email protected]> On Wed, Feb 18, 2026 at 10:51 AM Richard Guo <[email protected]> wrote: > Exactly. I think this is because when adjust_appendrel_attrs_mutator > propagates the nullingrel bits from the parent rel's Var into the > translated Var, it loses the translated Var's original bits. Instead > of overwriting the translated Var's nullingrels, I think we should > merge them. > > --- a/src/backend/optimizer/util/appendinfo.c > +++ b/src/backend/optimizer/util/appendinfo.c > @@ -291,8 +291,11 @@ adjust_appendrel_attrs_mutator(Node *node, > var->varattno, get_rel_name(appinfo->parent_reloid)); > if (IsA(newnode, Var)) > { > - ((Var *) newnode)->varreturningtype = var->varreturningtype; > - ((Var *) newnode)->varnullingrels = var->varnullingrels; > + Var *newvar = (Var *) newnode; > + > + newvar->varreturningtype = var->varreturningtype; > + newvar->varnullingrels = bms_add_members(newvar->varnullingrels, > + var->varnullingrels); > } Here is a more readable version of the patch. - Richard Attachments: [application/octet-stream] v1-0001-Fix-computation-of-varnullingrels-when-translatin.patch (5.3K, 2-v1-0001-Fix-computation-of-varnullingrels-when-translatin.patch) download | inline diff: From 5f473566b134ca0ff46165d2c7cdd9e0259ba56c Mon Sep 17 00:00:00 2001 From: Richard Guo <[email protected]> Date: Wed, 18 Feb 2026 17:25:06 +0900 Subject: [PATCH v1] Fix computation of varnullingrels when translating appendrel Var When adjust_appendrel_attrs translates a Var referencing a parent relation into a Var referencing a child relation, it propagates varnullingrels from the parent Var to the translated Var. Previously, the code simply overwrote the translated Var's varnullingrels with those of the parent. This was incorrect because the translated Var might already possess nonempty varnullingrels. This happens, for example, when a LATERAL subquery within a UNION ALL references a Var from the nullable side of an outer join. In such cases, the translated Var correctly carries the outer join's relid in its varnullingrels. Overwriting these bits with the parent Var's set caused the planner to lose track of the fact that the Var could be nulled by that outer join. In the reported case, because the underlying column had a NOT NULL constraint, the planner incorrectly deduced that the Var could never be NULL and discarded essential IS NOT NULL filters. This led to incorrect query results where NULL rows were returned instead of being filtered out. To fix, use bms_add_members to merge the parent Var's varnullingrels into the translated Var's existing set, preserving both sources of nullability. --- src/backend/optimizer/util/appendinfo.c | 13 ++++++- src/test/regress/expected/join.out | 51 +++++++++++++++++++++++++ src/test/regress/sql/join.sql | 24 ++++++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c index 689840d6564..2e7b1b202a3 100644 --- a/src/backend/optimizer/util/appendinfo.c +++ b/src/backend/optimizer/util/appendinfo.c @@ -291,8 +291,17 @@ adjust_appendrel_attrs_mutator(Node *node, var->varattno, get_rel_name(appinfo->parent_reloid)); if (IsA(newnode, Var)) { - ((Var *) newnode)->varreturningtype = var->varreturningtype; - ((Var *) newnode)->varnullingrels = var->varnullingrels; + Var *newvar = (Var *) newnode; + + newvar->varreturningtype = var->varreturningtype; + + /* + * Propagate var->varnullingrels into the translated Var, + * merging them with any nullingrels already present in + * the translated Var rather than overwriting them. + */ + newvar->varnullingrels = bms_add_members(newvar->varnullingrels, + var->varnullingrels); } else { diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 63d3c5d3ac8..072a7347b81 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -4486,6 +4486,57 @@ where ss.x is null; Output: 'bar'::text (12 rows) +-- Test computation of varnullingrels when translating appendrel Var +begin; +create temp table t_append (a int not null, b int); +insert into t_append values (1, 1); +insert into t_append values (2, 3); +explain (verbose, costs off) +select t1.a, s.a from t_append t1 + left join t_append t2 on t1.a = t2.b + join lateral ( + select t1.a as a union all select t2.a as a + ) s on true +where s.a is not null; + QUERY PLAN +--------------------------------------------------- + Nested Loop + Output: t1.a, (t1.a) + -> Merge Left Join + Output: t1.a, t2.a + Merge Cond: (t1.a = t2.b) + -> Sort + Output: t1.a + Sort Key: t1.a + -> Seq Scan on pg_temp.t_append t1 + Output: t1.a + -> Sort + Output: t2.a, t2.b + Sort Key: t2.b + -> Seq Scan on pg_temp.t_append t2 + Output: t2.a, t2.b + -> Append + -> Result + Output: t1.a + -> Result + Output: t2.a + One-Time Filter: (t2.a IS NOT NULL) +(21 rows) + +select t1.a, s.a from t_append t1 + left join t_append t2 on t1.a = t2.b + join lateral ( + select t1.a as a union all select t2.a as a + ) s on true +where s.a is not null; + a | a +---+--- + 1 | 1 + 1 | 1 + 2 | 2 +(3 rows) + +rollback; -- -- test inlining of immutable functions -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 14cbec28766..4acd2512004 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1508,6 +1508,30 @@ select * from int4_tbl left join ( ) ss(x) on true where ss.x is null; +-- Test computation of varnullingrels when translating appendrel Var +begin; + +create temp table t_append (a int not null, b int); +insert into t_append values (1, 1); +insert into t_append values (2, 3); + +explain (verbose, costs off) +select t1.a, s.a from t_append t1 + left join t_append t2 on t1.a = t2.b + join lateral ( + select t1.a as a union all select t2.a as a + ) s on true +where s.a is not null; + +select t1.a, s.a from t_append t1 + left join t_append t2 on t1.a = t2.b + join lateral ( + select t1.a as a union all select t2.a as a + ) s on true +where s.a is not null; + +rollback; + -- -- test inlining of immutable functions -- -- 2.39.5 (Apple Git-154) ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #19412: Wrong query result with not null constraint @ 2026-02-18 12:03 Sergey Shinderuk <[email protected]> parent: Richard Guo <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Sergey Shinderuk @ 2026-02-18 12:03 UTC (permalink / raw) To: Richard Guo <[email protected]>; +Cc: [email protected]; Tom Lane <[email protected]>; David Rowley <[email protected]> On 2/18/26 12:50, Richard Guo wrote: > On Wed, Feb 18, 2026 at 10:51 AM Richard Guo <[email protected]> wrote: >> Exactly. I think this is because when adjust_appendrel_attrs_mutator >> propagates the nullingrel bits from the parent rel's Var into the >> translated Var, it loses the translated Var's original bits. Instead >> of overwriting the translated Var's nullingrels, I think we should >> merge them. >> >> --- a/src/backend/optimizer/util/appendinfo.c >> +++ b/src/backend/optimizer/util/appendinfo.c >> @@ -291,8 +291,11 @@ adjust_appendrel_attrs_mutator(Node *node, >> var->varattno, get_rel_name(appinfo->parent_reloid)); >> if (IsA(newnode, Var)) >> { >> - ((Var *) newnode)->varreturningtype = var->varreturningtype; >> - ((Var *) newnode)->varnullingrels = var->varnullingrels; >> + Var *newvar = (Var *) newnode; >> + >> + newvar->varreturningtype = var->varreturningtype; >> + newvar->varnullingrels = bms_add_members(newvar->varnullingrels, >> + var->varnullingrels); >> } > > Here is a more readable version of the patch. > Thank you! I'm not familiar with the code, just curios. There is a long comment above saying "You might think we need to adjust var->varnullingrels, but that shouldn't need any changes." Doesn't it need an update? -- Sergey Shinderuk https://postgrespro.com/ ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #19412: Wrong query result with not null constraint @ 2026-02-19 01:49 Richard Guo <[email protected]> parent: Sergey Shinderuk <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Richard Guo @ 2026-02-19 01:49 UTC (permalink / raw) To: Sergey Shinderuk <[email protected]>; +Cc: [email protected]; Tom Lane <[email protected]>; David Rowley <[email protected]> On Wed, Feb 18, 2026 at 9:03 PM Sergey Shinderuk <[email protected]> wrote: > I'm not familiar with the code, just curios. There is a long comment > above saying "You might think we need to adjust var->varnullingrels, but > that shouldn't need any changes." Doesn't it need an update? No, I don't think we need to update it. That comment explains why varnullingrels do not require translation (since they are outer join relids, not baserel relids). It's unrelated to what this patch does, which is about propagating varnullingrels into the translated Var. - Richard ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #19412: Wrong query result with not null constraint @ 2026-02-19 18:25 Tom Lane <[email protected]> parent: Richard Guo <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Tom Lane @ 2026-02-19 18:25 UTC (permalink / raw) To: Richard Guo <[email protected]>; +Cc: Sergey Shinderuk <[email protected]>; [email protected]; David Rowley <[email protected]> Richard Guo <[email protected]> writes: > On Wed, Feb 18, 2026 at 9:03 PM Sergey Shinderuk > <[email protected]> wrote: >> I'm not familiar with the code, just curios. There is a long comment >> above saying "You might think we need to adjust var->varnullingrels, but >> that shouldn't need any changes." Doesn't it need an update? > No, I don't think we need to update it. That comment explains why > varnullingrels do not require translation (since they are outer join > relids, not baserel relids). It's unrelated to what this patch does, > which is about propagating varnullingrels into the translated Var. I agree with this fix: I think the code is like it is simply because it didn't occur to me that the child Vars could have any nullingrel bits yet. However, I don't agree that that comment needs no updates. I suggest something like - * Below, we just propagate var->varnullingrels into the translated - * Var. + * Below, we just merge var->varnullingrels into the translated + * Var. (We must merge not just copy: the child Var could have + * some nullingrel bits set already, and we mustn't drop those.) Also, I think I'd then drop the comment you added adjacent to the actual update; it seems redundant if the earlier comment says this. I agree with back-patching to v16. This particular example doesn't misbehave in versions that don't have the drop-allegedly-redundant- NOT-NULL-tests logic, but the varnullingrels are certainly wrong all the way back, so possibly there are other examples that do misbehave in v16. regards, tom lane ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #19412: Wrong query result with not null constraint @ 2026-02-20 09:57 Richard Guo <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Richard Guo @ 2026-02-20 09:57 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Sergey Shinderuk <[email protected]>; [email protected]; David Rowley <[email protected]> On Fri, Feb 20, 2026 at 3:25 AM Tom Lane <[email protected]> wrote: > I agree with this fix: I think the code is like it is simply because > it didn't occur to me that the child Vars could have any nullingrel > bits yet. However, I don't agree that that comment needs no updates. > I suggest something like > > - * Below, we just propagate var->varnullingrels into the translated > - * Var. > + * Below, we just merge var->varnullingrels into the translated > + * Var. (We must merge not just copy: the child Var could have > + * some nullingrel bits set already, and we mustn't drop those.) > > Also, I think I'd then drop the comment you added adjacent to the > actual update; it seems redundant if the earlier comment says this. Thanks for taking a look. I've updated the comments as suggested, > I agree with back-patching to v16. This particular example doesn't > misbehave in versions that don't have the drop-allegedly-redundant- > NOT-NULL-tests logic, but the varnullingrels are certainly wrong > all the way back, so possibly there are other examples that do > misbehave in v16. ... and then pushed and back-patched to v16. Thank you, Sergey, for the report and the excellent self-contained repro query. This is a great catch. (I'm curious how you found this bug -- was it from a production query or a fuzzing tool?) - Richard ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #19412: Wrong query result with not null constraint @ 2026-02-20 10:54 Sergey Shinderuk <[email protected]> parent: Richard Guo <[email protected]> 0 siblings, 0 replies; 8+ messages in thread From: Sergey Shinderuk @ 2026-02-20 10:54 UTC (permalink / raw) To: Richard Guo <[email protected]>; +Cc: [email protected]; David Rowley <[email protected]>; Tom Lane <[email protected]> On 2/20/26 12:57, Richard Guo wrote: > ... and then pushed and back-patched to v16. Thank you! > Thank you, Sergey, for the report and the excellent self-contained > repro query. This is a great catch. (I'm curious how you found this > bug -- was it from a production query or a fuzzing tool?) The bug was encountered by our customer upgrading to v17 and my colleague passed this repro to me. -- Sergey Shinderuk https://postgrespro.com/ ^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2026-02-20 10:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-02-17 22:53 Re: BUG #19412: Wrong query result with not null constraint David Rowley <[email protected]> 2026-02-18 01:51 ` Richard Guo <[email protected]> 2026-02-18 09:50 ` Richard Guo <[email protected]> 2026-02-18 12:03 ` Sergey Shinderuk <[email protected]> 2026-02-19 01:49 ` Richard Guo <[email protected]> 2026-02-19 18:25 ` Tom Lane <[email protected]> 2026-02-20 09:57 ` Richard Guo <[email protected]> 2026-02-20 10:54 ` Sergey Shinderuk <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox