Fix var_is_nonnullable() to account for varreturningtype

var_is_nonnullable() failed to consider varreturningtype, which meant
it could incorrectly claim a Var is non-nullable based on a column's
NOT NULL constraint even when the Var refers to a non-existent row.
Specifically, OLD.col is NULL for INSERT (no old row exists) and
NEW.col is NULL for DELETE (no new row exists), regardless of any NOT
NULL constraint on the column.

This caused the planner's constant folding in eval_const_expressions
to incorrectly simplify IS NULL / IS NOT NULL tests on such Vars.  For
example, "old.a IS NULL" in an INSERT's RETURNING clause would be
folded to false when column "a" has a NOT NULL constraint, even though
the correct result is true.

Fix by returning false from var_is_nonnullable() when varreturningtype
is not VAR_RETURNING_DEFAULT, since such Vars can be NULL regardless
of table constraints.

Author: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAHg+QDfaAipL6YzOq2H=gAhKBbcUTYmfbAv+W1zueOfRKH43FQ@mail.gmail.com

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 9fb266d089dfcc8b0d7d7eb9a8229ed0dc961f31..cd4e2e86c6d988a76e0d7071e96b087aa60620ff 100644 (file)
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4635,6 +4635,14 @@ var_is_nonnullable(PlannerInfo *root, Var *var, NotNullSource source)
        if (!bms_is_empty(var->varnullingrels))
                return false;
 
+       /*
+        * If the Var has a non-default returning type, it could be NULL
+        * regardless of any NOT NULL constraint.  For example, OLD.col is NULL
+        * for INSERT, and NEW.col is NULL for DELETE.
+        */
+       if (var->varreturningtype != VAR_RETURNING_DEFAULT)
+               return false;
+
        /* system columns cannot be NULL */
        if (var->varattno < 0)
                return true;

diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out
index cfaaf015bb388cfe311028fa48fd8ef3d1549c98..196829e94fa4b47a91ed9fbdcca72bfdb4c64320 100644 (file)
--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -990,3 +990,34 @@ BEGIN ATOMIC
            WHERE (foo_1.* = n.*)) AS count;
 END
 DROP FUNCTION foo_update;
+-- Test that the planner does not fold OLD/NEW IS NULL tests to constants
+-- based on NOT NULL constraints, since OLD is NULL for INSERT and NEW is
+-- NULL for DELETE.
+CREATE TEMP TABLE ret_nn (a int NOT NULL);
+-- INSERT has no OLD row, should return true
+INSERT INTO ret_nn VALUES (1) RETURNING old.a IS NULL;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- DELETE has no NEW row, should return true
+DELETE FROM ret_nn WHERE a = 1 RETURNING new.a IS NULL;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- MERGE: DELETE should have new.a IS NULL, INSERT should have old.a IS NULL
+INSERT INTO ret_nn VALUES (2);
+MERGE INTO ret_nn USING (VALUES (2), (3)) AS src(a) ON ret_nn.a = src.a
+  WHEN MATCHED THEN DELETE
+  WHEN NOT MATCHED THEN INSERT VALUES (src.a)
+  RETURNING merge_action(), old.a IS NULL, new.a IS NULL;
+ merge_action | ?column? | ?column? 
+--------------+----------+----------
+ DELETE       | f        | t
+ INSERT       | t        | f
+(2 rows)
+
+DROP TABLE ret_nn;

diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sql
index cc99cb53f63c4b87e152af838e62da53e29d9e93..b3c8c5df550c8648c259076fa955f34415c2b032 100644 (file)
--- a/src/test/regress/sql/returning.sql
+++ b/src/test/regress/sql/returning.sql
@@ -408,3 +408,23 @@ END;
 
 \sf foo_update
 DROP FUNCTION foo_update;
+
+-- Test that the planner does not fold OLD/NEW IS NULL tests to constants
+-- based on NOT NULL constraints, since OLD is NULL for INSERT and NEW is
+-- NULL for DELETE.
+CREATE TEMP TABLE ret_nn (a int NOT NULL);
+
+-- INSERT has no OLD row, should return true
+INSERT INTO ret_nn VALUES (1) RETURNING old.a IS NULL;
+
+-- DELETE has no NEW row, should return true
+DELETE FROM ret_nn WHERE a = 1 RETURNING new.a IS NULL;
+
+-- MERGE: DELETE should have new.a IS NULL, INSERT should have old.a IS NULL
+INSERT INTO ret_nn VALUES (2);
+MERGE INTO ret_nn USING (VALUES (2), (3)) AS src(a) ON ret_nn.a = src.a
+  WHEN MATCHED THEN DELETE
+  WHEN NOT MATCHED THEN INSERT VALUES (src.a)
+  RETURNING merge_action(), old.a IS NULL, new.a IS NULL;
+
+DROP TABLE ret_nn;