public inbox for [email protected]  
help / color / mirror / Atom feed
From: SATYANARAYANA NARLAPURAM <[email protected]>
To: PostgreSQL-development <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING
Date: Thu, 9 Apr 2026 11:43:09 -0700
Message-ID: <CAHg+QDfaAipL6YzOq2H=gAhKBbcUTYmfbAv+W1zueOfRKH43FQ@mail.gmail.com> (raw)

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;


view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING
  In-Reply-To: <CAHg+QDfaAipL6YzOq2H=gAhKBbcUTYmfbAv+W1zueOfRKH43FQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox