public inbox for [email protected]
help / color / mirror / Atom feedBug: var_is_nonnullable() gives wrong results for old/new in RETURNING
4+ messages / 3 participants
[nested] [flat]
* Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING
@ 2026-04-09 18:43 SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-10 01:29 ` Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING Tender Wang <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-04-09 18:43 UTC (permalink / raw)
To: pgsql-hackers; PostgreSQL Hackers <[email protected]>
Hi hackers,
It appears the optimizer incorrectly simplifies old.<col> IS NULL to FALSE
in RETURNING clauses when the underlying column has a NOT NULL constraint.
The issue is that var_is_nonnullable() in clauses.c doesn't check
Var.varreturningtype. It sees a NOT NULL column and concludes the Var can
never be NULL.
But this assumption is wrong for old.* and new.* references. Because the
old tuple doesn't exist on INSERT, and the new tuple doesn't exist on
DELETE
I am not super familiar with this area, so I attempted to fix this as in
the patch attached.
Repro:
postgres=# CREATE TABLE t (id INT NOT NULL PRIMARY KEY, val INT);
INSERT INTO t VALUES (1, 10);
MERGE INTO t
USING (VALUES (1, 99), (2, 50)) AS s(id, val) ON t.id = s.id
WHEN MATCHED THEN UPDATE SET val = s.val
WHEN NOT MATCHED THEN INSERT VALUES (s.id, s.val)
RETURNING merge_action(),
old.id IS NULL AS is_new_row;
CREATE TABLE
INSERT 0 1
merge_action | is_new_row
--------------+------------
UPDATE | f
INSERT | f -- (this should be true)
(2 rows)
MERGE 2
Thanks,
Satya
Attachments:
[application/octet-stream] 0001-test-var_is_nonnullable-returning-old-new.patch (2.4K, 3-0001-test-var_is_nonnullable-returning-old-new.patch)
download | inline diff:
diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/merge.out
index 9cb1d870..4441e399 100644
--- a/src/test/regress/expected/merge.out
+++ b/src/test/regress/expected/merge.out
@@ -1609,6 +1609,31 @@ LATERAL (SELECT r_action, r_tid, r_balance FROM merge_into_sq_target(sid, balanc
INSERT | 4 | 110
(3 rows)
+ROLLBACK;
+-- Test that old/new column IS NULL is constant-folded
+-- for NOT NULL columns in MERGE RETURNING
+BEGIN;
+MERGE INTO sq_target t
+USING sq_source s ON tid = sid
+WHEN MATCHED AND tid >= 2 THEN
+ UPDATE SET balance = t.balance + delta
+WHEN NOT MATCHED THEN
+ INSERT (balance, tid) VALUES (balance + delta, sid)
+WHEN MATCHED AND tid < 2 THEN
+ DELETE
+RETURNING merge_action(),
+ old.tid IS NULL AS old_tid_is_null,
+ new.tid IS NULL AS new_tid_is_null,
+ old IS NULL AS old_row_is_null,
+ new IS NULL AS new_row_is_null;
+ merge_action | old_tid_is_null | new_tid_is_null | old_row_is_null | new_row_is_null
+--------------+-----------------+-----------------+-----------------+-----------------
+ DELETE | f | t | f | t
+ UPDATE | f | f | f | f
+ INSERT | t | f | t | f
+(3 rows)
+
ROLLBACK;
-- EXPLAIN
CREATE TABLE ex_mtarget (a int, b int)
diff --git a/src/test/regress/sql/merge.sql b/src/test/regress/sql/merge.sql
index 2660b19f..6789fcd5 100644
--- a/src/test/regress/sql/merge.sql
+++ b/src/test/regress/sql/merge.sql
@@ -1060,6 +1060,25 @@ FROM (VALUES (1, 0, 0), (3, 0, 20), (4, 100, 10)) AS v(sid, balance, delta),
LATERAL (SELECT r_action, r_tid, r_balance FROM merge_into_sq_target(sid, balance, delta)) m;
ROLLBACK;
+-- Test that old/new column IS NULL is constant-folded
+-- for NOT NULL columns in MERGE RETURNING
+BEGIN;
+MERGE INTO sq_target t
+USING sq_source s ON tid = sid
+WHEN MATCHED AND tid >= 2 THEN
+ UPDATE SET balance = t.balance + delta
+WHEN NOT MATCHED THEN
+ INSERT (balance, tid) VALUES (balance + delta, sid)
+WHEN MATCHED AND tid < 2 THEN
+ DELETE
+RETURNING merge_action(),
+ old.tid IS NULL AS old_tid_is_null,
+ new.tid IS NULL AS new_tid_is_null,
+ old IS NULL AS old_row_is_null,
+ new IS NULL AS new_row_is_null;
+ROLLBACK;
+
-- EXPLAIN
CREATE TABLE ex_mtarget (a int, b int)
WITH (autovacuum_enabled=off);
[application/octet-stream] 0001-fix-var_is_nonnullable-returning-old-new.patch (848B, 4-0001-fix-var_is_nonnullable-returning-old-new.patch)
download | inline diff:
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 9fb266d0..f290a439 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4631,6 +4631,15 @@ var_is_nonnullable(PlannerInfo *root, Var *var, NotNullSource source)
if (var->varlevelsup != 0)
return false;
+ /*
+ * A Var with VAR_RETURNING_OLD or VAR_RETURNING_NEW refers to the OLD or
+ * NEW tuple in a RETURNING list. Such a Var can be NULL even if the
+ * underlying column is defined NOT NULL, because the OLD tuple doesn't
+ * exist for INSERT and the NEW tuple doesn't exist for DELETE.
+ */
+ if (var->varreturningtype != VAR_RETURNING_DEFAULT)
+ return false;
+
/* could the Var be nulled by any outer joins or grouping sets? */
if (!bms_is_empty(var->varnullingrels))
return false;
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING
2026-04-09 18:43 Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING SATYANARAYANA NARLAPURAM <[email protected]>
@ 2026-04-10 01:29 ` Tender Wang <[email protected]>
2026-04-10 05:48 ` Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING Richard Guo <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Tender Wang @ 2026-04-10 01:29 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Richard Guo <[email protected]>
SATYANARAYANA NARLAPURAM <[email protected]> 于2026年4月10日周五 02:43写道：
>
> Hi hackers,
>
> It appears the optimizer incorrectly simplifies old.<col> IS NULL to FALSE in RETURNING clauses when the underlying column has a NOT NULL constraint.
>
> The issue is that var_is_nonnullable() in clauses.c doesn't check Var.varreturningtype. It sees a NOT NULL column and concludes the Var can never be NULL.
> But this assumption is wrong for old.* and new.* references. Because the old tuple doesn't exist on INSERT, and the new tuple doesn't exist on DELETE
> I am not super familiar with this area, so I attempted to fix this as in the patch attached.
Yes, the current var_is_nonnullable() ignores this case. The
attached patch seems ok to me.
Add Richard to the cc list. He may know more about this.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING
2026-04-09 18:43 Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-10 01:29 ` Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING Tender Wang <[email protected]>
@ 2026-04-10 05:48 ` Richard Guo <[email protected]>
2026-04-10 06:58 ` Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING Richard Guo <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Richard Guo @ 2026-04-10 05:48 UTC (permalink / raw)
To: Tender Wang <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Apr 10, 2026 at 10:30 AM Tender Wang <[email protected]> wrote:
> SATYANARAYANA NARLAPURAM <[email protected]> 于2026年4月10日周五 02:43写道：
> > It appears the optimizer incorrectly simplifies old.<col> IS NULL to FALSE in RETURNING clauses when the underlying column has a NOT NULL constraint.
> >
> > The issue is that var_is_nonnullable() in clauses.c doesn't check Var.varreturningtype. It sees a NOT NULL column and concludes the Var can never be NULL.
> > But this assumption is wrong for old.* and new.* references. Because the old tuple doesn't exist on INSERT, and the new tuple doesn't exist on DELETE
Nice catch.
> Yes, the current var_is_nonnullable() ignores this case. The
> attached patch seems ok to me.
The patch also LGTM. I also checked if has_notnull_forced_var() has
the same issue, but it doesn't: Vars with non-default returning type
only appear in the RETURNING clause, so they never show up in WHERE/ON
clauses.
- Richard
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING
2026-04-09 18:43 Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-10 01:29 ` Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING Tender Wang <[email protected]>
2026-04-10 05:48 ` Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING Richard Guo <[email protected]>
@ 2026-04-10 06:58 ` Richard Guo <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Richard Guo @ 2026-04-10 06:58 UTC (permalink / raw)
To: Tender Wang <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Apr 10, 2026 at 2:48 PM Richard Guo <[email protected]> wrote:
> The patch also LGTM. I also checked if has_notnull_forced_var() has
> the same issue, but it doesn't: Vars with non-default returning type
> only appear in the RETURNING clause, so they never show up in WHERE/ON
> clauses.
I pushed the patch after a bit cosmetic tweaks. I also decided to put
the test cases in returning.sql, which I think is a better place.
Thanks for the report and the patch.
- Richard
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-04-10 06:58 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-09 18:43 Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-10 01:29 ` Tender Wang <[email protected]>
2026-04-10 05:48 ` Richard Guo <[email protected]>
2026-04-10 06:58 ` Richard Guo <[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