]> granicus.if.org Git - postgresql/commitdiff
Fix constant-folding of ROW(...) IS [NOT] NULL with composite fields.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 26 Jul 2016 19:25:02 +0000 (15:25 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 26 Jul 2016 19:25:02 +0000 (15:25 -0400)
The SQL standard appears to specify that IS [NOT] NULL's tests of field
nullness are non-recursive, ie, we shouldn't consider that a composite
field with value ROW(NULL,NULL) is null for this purpose.
ExecEvalNullTest got this right, but eval_const_expressions did not,
leading to weird inconsistencies depending on whether the expression
was such that the planner could apply constant folding.

Also, adjust the docs to mention that IS [NOT] DISTINCT FROM NULL can be
used as a substitute test if a simple null check is wanted for a rowtype
argument.  That motivated reordering things so that IS [NOT] DISTINCT FROM
is described before IS [NOT] NULL.  In HEAD, I went a bit further and added
a table showing all the comparison-related predicates.

Per bug #14235.  Back-patch to all supported branches, since it's certainly
undesirable that constant-folding should change the semantics.

Report and patch by Andrew Gierth; assorted wordsmithing and revised
regression test cases by me.

Report: <20160708024746.1410.57282@wrigleys.postgresql.org>

doc/src/sgml/func.sgml
src/backend/executor/execQual.c
src/backend/optimizer/util/clauses.c
src/test/regress/expected/rowtypes.out
src/test/regress/sql/rowtypes.sql

index 9482f6a72ee1b50563fab29888ec4eac865d039d..c097e8056cd39e7bba53c05d989d2e45c7881386 100644 (file)
     a nonempty range is always implied.
    </para>
 
+   <para>
+    <indexterm>
+     <primary>IS DISTINCT FROM</primary>
+    </indexterm>
+    <indexterm>
+     <primary>IS NOT DISTINCT FROM</primary>
+    </indexterm>
+    Ordinary comparison operators yield null (signifying <quote>unknown</>),
+    not true or false, when either input is null.  For example,
+    <literal>7 = NULL</> yields null, as does <literal>7 &lt;&gt; NULL</>.  When
+    this behavior is not suitable, use the
+    <literal>IS <optional> NOT </> DISTINCT FROM</literal> constructs:
+<synopsis>
+<replaceable>a</replaceable> IS DISTINCT FROM <replaceable>b</replaceable>
+<replaceable>a</replaceable> IS NOT DISTINCT FROM <replaceable>b</replaceable>
+</synopsis>
+    For non-null inputs, <literal>IS DISTINCT FROM</literal> is
+    the same as the <literal>&lt;&gt;</> operator.  However, if both
+    inputs are null it returns false, and if only one input is
+    null it returns true.  Similarly, <literal>IS NOT DISTINCT
+    FROM</literal> is identical to <literal>=</literal> for non-null
+    inputs, but it returns true when both inputs are null, and false when only
+    one input is null. Thus, these constructs effectively act as though null
+    were a normal data value, rather than <quote>unknown</>.
+   </para>
+
    <para>
     <indexterm>
      <primary>IS NULL</primary>
     <literal><replaceable>expression</replaceable> = NULL</literal>
     because <literal>NULL</> is not <quote>equal to</quote>
     <literal>NULL</>.  (The null value represents an unknown value,
-    and it is not known whether two unknown values are equal.) This
-    behavior conforms to the SQL standard.
+    and it is not known whether two unknown values are equal.)
    </para>
 
   <tip>
    </para>
   </tip>
 
-  <note>
    <para>
     If the <replaceable>expression</replaceable> is row-valued, then
     <literal>IS NULL</> is true when the row expression itself is null
     <literal>IS NOT NULL</> is true when the row expression itself is non-null
     and all the row's fields are non-null.  Because of this behavior,
     <literal>IS NULL</> and <literal>IS NOT NULL</> do not always return
-    inverse results for row-valued expressions, i.e., a row-valued
-    expression that contains both NULL and non-null values will return false
-    for both tests.
-    This definition conforms to the SQL standard, and is a change from the
-    inconsistent behavior exhibited by <productname>PostgreSQL</productname>
-    versions prior to 8.2.
-   </para>
-  </note>
-
-   <para>
-    <indexterm>
-     <primary>IS DISTINCT FROM</primary>
-    </indexterm>
-    <indexterm>
-     <primary>IS NOT DISTINCT FROM</primary>
-    </indexterm>
-    Ordinary comparison operators yield null (signifying <quote>unknown</>),
-    not true or false, when either input is null.  For example,
-    <literal>7 = NULL</> yields null.  When this behavior is not suitable,
-    use the
-    <literal>IS <optional> NOT </> DISTINCT FROM</literal> constructs:
-<synopsis>
-<replaceable>expression</replaceable> IS DISTINCT FROM <replaceable>expression</replaceable>
-<replaceable>expression</replaceable> IS NOT DISTINCT FROM <replaceable>expression</replaceable>
-</synopsis>
-    For non-null inputs, <literal>IS DISTINCT FROM</literal> is
-    the same as the <literal>&lt;&gt;</> operator.  However, if both
-    inputs are null it returns false, and if only one input is
-    null it returns true.  Similarly, <literal>IS NOT DISTINCT
-    FROM</literal> is identical to <literal>=</literal> for non-null
-    inputs, but it returns true when both inputs are null, and false when only
-    one input is null. Thus, these constructs effectively act as though null
-    were a normal data value, rather than <quote>unknown</>.
+    inverse results for row-valued expressions; in particular, a row-valued
+    expression that contains both null and non-null fields will return false
+    for both tests.  In some cases, it may be preferable to
+    write <replaceable>row</replaceable> <literal>IS DISTINCT FROM NULL</>
+    or <replaceable>row</replaceable> <literal>IS NOT DISTINCT FROM NULL</>,
+    which will simply check whether the overall row value is null without any
+    additional tests on the row fields.
    </para>
 
    <para>
index ebc6a3f150007d9c7c6ffa8f10ab68e6da443ad9..ea8850e12cff07f29cb99144c5e665ca36c8b3c5 100644 (file)
@@ -3701,6 +3701,21 @@ ExecEvalNullTest(NullTestState *nstate,
 
        if (ntest->argisrow && !(*isNull))
        {
+               /*
+                * The SQL standard defines IS [NOT] NULL for a non-null rowtype
+                * argument as:
+                *
+                * "R IS NULL" is true if every field is the null value.
+                *
+                * "R IS NOT NULL" is true if no field is the null value.
+                *
+                * This definition is (apparently intentionally) not recursive; so our
+                * tests on the fields are primitive attisnull tests, not recursive
+                * checks to see if they are all-nulls or no-nulls rowtypes.
+                *
+                * The standard does not consider the possibility of zero-field rows,
+                * but here we consider them to vacuously satisfy both predicates.
+                */
                HeapTupleHeader tuple;
                Oid                     tupType;
                int32           tupTypmod;
index e7bfc38d5669044902b3bc28b9d23b9aba6d1ee4..667e682e2d17f844f8596d982d07d321a885e48a 100644 (file)
@@ -2970,7 +2970,7 @@ eval_const_expressions_mutator(Node *node,
 
                arg = eval_const_expressions_mutator((Node *) ntest->arg,
                                                                                         context);
-               if (arg && IsA(arg, RowExpr))
+               if (ntest->argisrow && arg && IsA(arg, RowExpr))
                {
                        /*
                         * We break ROW(...) IS [NOT] NULL into separate tests on its
@@ -2981,8 +2981,6 @@ eval_const_expressions_mutator(Node *node,
                        List       *newargs = NIL;
                        ListCell   *l;
 
-                       Assert(ntest->argisrow);
-
                        foreach(l, rarg->args)
                        {
                                Node       *relem = (Node *) lfirst(l);
@@ -3001,10 +2999,16 @@ eval_const_expressions_mutator(Node *node,
                                                return makeBoolConst(false, false);
                                        continue;
                                }
+
+                               /*
+                                * Else, make a scalar (argisrow == false) NullTest for this
+                                * field.  Scalar semantics are required because IS [NOT] NULL
+                                * doesn't recurse; see comments in ExecEvalNullTest().
+                                */
                                newntest = makeNode(NullTest);
                                newntest->arg = (Expr *) relem;
                                newntest->nulltesttype = ntest->nulltesttype;
-                               newntest->argisrow = type_is_rowtype(exprType(relem));
+                               newntest->argisrow = false;
                                newargs = lappend(newargs, newntest);
                        }
                        /* If all the inputs were constants, result is TRUE */
index 96da4be048cd140301f92e0f16294c33336dc288..93301992c27316e61f196b70f530e0efc8da5ccd 100644 (file)
@@ -442,3 +442,57 @@ select (row('Jim', 'Beam')).text;  -- error
 ERROR:  could not identify column "text" in record data type
 LINE 1: select (row('Jim', 'Beam')).text;
                 ^
+--
+-- IS [NOT] NULL should not recurse into nested composites (bug #14235)
+--
+explain (verbose, costs off)
+select r, r is null as isnull, r is not null as isnotnull
+from (values (1,row(1,2)), (1,row(null,null)), (1,null),
+             (null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
+                                                                                           QUERY PLAN                                                                                            
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Values Scan on "*VALUES*"
+   Output: ROW("*VALUES*".column1, "*VALUES*".column2), (("*VALUES*".column1 IS NULL) AND ("*VALUES*".column2 IS NULL)), (("*VALUES*".column1 IS NOT NULL) AND ("*VALUES*".column2 IS NOT NULL))
+(2 rows)
+
+select r, r is null as isnull, r is not null as isnotnull
+from (values (1,row(1,2)), (1,row(null,null)), (1,null),
+             (null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
+      r      | isnull | isnotnull 
+-------------+--------+-----------
+ (1,"(1,2)") | f      | t
+ (1,"(,)")   | f      | t
+ (1,)        | f      | f
+ (,"(1,2)")  | f      | f
+ (,"(,)")    | f      | f
+ (,)         | t      | f
+(6 rows)
+
+explain (verbose, costs off)
+with r(a,b) as
+  (values (1,row(1,2)), (1,row(null,null)), (1,null),
+          (null,row(1,2)), (null,row(null,null)), (null,null) )
+select r, r is null as isnull, r is not null as isnotnull from r;
+                        QUERY PLAN                        
+----------------------------------------------------------
+ CTE Scan on r
+   Output: r.*, (r.* IS NULL), (r.* IS NOT NULL)
+   CTE r
+     ->  Values Scan on "*VALUES*"
+           Output: "*VALUES*".column1, "*VALUES*".column2
+(5 rows)
+
+with r(a,b) as
+  (values (1,row(1,2)), (1,row(null,null)), (1,null),
+          (null,row(1,2)), (null,row(null,null)), (null,null) )
+select r, r is null as isnull, r is not null as isnotnull from r;
+      r      | isnull | isnotnull 
+-------------+--------+-----------
+ (1,"(1,2)") | f      | t
+ (1,"(,)")   | f      | t
+ (1,)        | f      | f
+ (,"(1,2)")  | f      | f
+ (,"(,)")    | f      | f
+ (,)         | t      | f
+(6 rows)
+
index 69851ec2ff12f2cbcff49dca0ff5fd94596eba5b..efe94f412cd3bc96699830bf4ad62470912cc2fb 100644 (file)
@@ -209,3 +209,27 @@ select cast (row('Jim', 'Beam') as text);
 select (row('Jim', 'Beam'))::text;
 select text(row('Jim', 'Beam'));  -- error
 select (row('Jim', 'Beam')).text;  -- error
+
+--
+-- IS [NOT] NULL should not recurse into nested composites (bug #14235)
+--
+
+explain (verbose, costs off)
+select r, r is null as isnull, r is not null as isnotnull
+from (values (1,row(1,2)), (1,row(null,null)), (1,null),
+             (null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
+
+select r, r is null as isnull, r is not null as isnotnull
+from (values (1,row(1,2)), (1,row(null,null)), (1,null),
+             (null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
+
+explain (verbose, costs off)
+with r(a,b) as
+  (values (1,row(1,2)), (1,row(null,null)), (1,null),
+          (null,row(1,2)), (null,row(null,null)), (null,null) )
+select r, r is null as isnull, r is not null as isnotnull from r;
+
+with r(a,b) as
+  (values (1,row(1,2)), (1,row(null,null)), (1,null),
+          (null,row(1,2)), (null,row(null,null)), (null,null) )
+select r, r is null as isnull, r is not null as isnotnull from r;