Skip to content

Commit c235d51

Browse files
tglsfdctglsfdc
committed
Fix constant-folding of ROW(...) IS [NOT] NULL with composite fields.
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>
1 parent 98b7a3c commit c235d51

File tree

5 files changed

+136
-40
lines changed

5 files changed

+136
-40
lines changed

‎doc/src/sgml/func.sgml

Lines changed: 34 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,32 @@
289289
a nonempty range is always implied.
290290
</para>
291291

292+
<para>
293+
<indexterm>
294+
<primary>IS DISTINCT FROM</primary>
295+
</indexterm>
296+
<indexterm>
297+
<primary>IS NOT DISTINCT FROM</primary>
298+
</indexterm>
299+
Ordinary comparison operators yield null (signifying <quote>unknown</>),
300+
not true or false, when either input is null. For example,
301+
<literal>7 = NULL</> yields null, as does <literal>7 &lt;&gt; NULL</>. When
302+
this behavior is not suitable, use the
303+
<literal>IS <optional> NOT </> DISTINCT FROM</literal> constructs:
304+
<synopsis>
305+
<replaceable>a</replaceable> IS DISTINCT FROM <replaceable>b</replaceable>
306+
<replaceable>a</replaceable> IS NOT DISTINCT FROM <replaceable>b</replaceable>
307+
</synopsis>
308+
For non-null inputs, <literal>IS DISTINCT FROM</literal> is
309+
the same as the <literal>&lt;&gt;</> operator. However, if both
310+
inputs are null it returns false, and if only one input is
311+
null it returns true. Similarly, <literal>IS NOT DISTINCT
312+
FROM</literal> is identical to <literal>=</literal> for non-null
313+
inputs, but it returns true when both inputs are null, and false when only
314+
one input is null. Thus, these constructs effectively act as though null
315+
were a normal data value, rather than <quote>unknown</>.
316+
</para>
317+
292318
<para>
293319
<indexterm>
294320
<primary>IS NULL</primary>
@@ -320,8 +346,7 @@
320346
<literal><replaceable>expression</replaceable> = NULL</literal>
321347
because <literal>NULL</> is not <quote>equal to</quote>
322348
<literal>NULL</>. (The null value represents an unknown value,
323-
and it is not known whether two unknown values are equal.) This
324-
behavior conforms to the SQL standard.
349+
and it is not known whether two unknown values are equal.)
325350
</para>
326351

327352
<tip>
@@ -338,47 +363,20 @@
338363
</para>
339364
</tip>
340365

341-
<note>
342366
<para>
343367
If the <replaceable>expression</replaceable> is row-valued, then
344368
<literal>IS NULL</> is true when the row expression itself is null
345369
or when all the row's fields are null, while
346370
<literal>IS NOT NULL</> is true when the row expression itself is non-null
347371
and all the row's fields are non-null. Because of this behavior,
348372
<literal>IS NULL</> and <literal>IS NOT NULL</> do not always return
349-
inverse results for row-valued expressions, i.e., a row-valued
350-
expression that contains both NULL and non-null values will return false
351-
for both tests.
352-
This definition conforms to the SQL standard, and is a change from the
353-
inconsistent behavior exhibited by <productname>PostgreSQL</productname>
354-
versions prior to 8.2.
355-
</para>
356-
</note>
357-
358-
<para>
359-
<indexterm>
360-
<primary>IS DISTINCT FROM</primary>
361-
</indexterm>
362-
<indexterm>
363-
<primary>IS NOT DISTINCT FROM</primary>
364-
</indexterm>
365-
Ordinary comparison operators yield null (signifying <quote>unknown</>),
366-
not true or false, when either input is null. For example,
367-
<literal>7 = NULL</> yields null, as does <literal>7 &lt;&gt; NULL</>. When
368-
this behavior is not suitable, use the
369-
<literal>IS <optional> NOT </> DISTINCT FROM</literal> constructs:
370-
<synopsis>
371-
<replaceable>expression</replaceable> IS DISTINCT FROM <replaceable>expression</replaceable>
372-
<replaceable>expression</replaceable> IS NOT DISTINCT FROM <replaceable>expression</replaceable>
373-
</synopsis>
374-
For non-null inputs, <literal>IS DISTINCT FROM</literal> is
375-
the same as the <literal>&lt;&gt;</> operator. However, if both
376-
inputs are null it returns false, and if only one input is
377-
null it returns true. Similarly, <literal>IS NOT DISTINCT
378-
FROM</literal> is identical to <literal>=</literal> for non-null
379-
inputs, but it returns true when both inputs are null, and false when only
380-
one input is null. Thus, these constructs effectively act as though null
381-
were a normal data value, rather than <quote>unknown</>.
373+
inverse results for row-valued expressions; in particular, a row-valued
374+
expression that contains both null and non-null fields will return false
375+
for both tests. In some cases, it may be preferable to
376+
write <replaceable>row</replaceable> <literal>IS DISTINCT FROM NULL</>
377+
or <replaceable>row</replaceable> <literal>IS NOT DISTINCT FROM NULL</>,
378+
which will simply check whether the overall row value is null without any
379+
additional tests on the row fields.
382380
</para>
383381

384382
<para>

‎src/backend/executor/execQual.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3757,6 +3757,21 @@ ExecEvalNullTest(NullTestState *nstate,
37573757

37583758
if (ntest->argisrow && !(*isNull))
37593759
{
3760+
/*
3761+
* The SQL standard defines IS [NOT] NULL for a non-null rowtype
3762+
* argument as:
3763+
*
3764+
* "R IS NULL" is true if every field is the null value.
3765+
*
3766+
* "R IS NOT NULL" is true if no field is the null value.
3767+
*
3768+
* This definition is (apparently intentionally) not recursive; so our
3769+
* tests on the fields are primitive attisnull tests, not recursive
3770+
* checks to see if they are all-nulls or no-nulls rowtypes.
3771+
*
3772+
* The standard does not consider the possibility of zero-field rows,
3773+
* but here we consider them to vacuously satisfy both predicates.
3774+
*/
37603775
HeapTupleHeader tuple;
37613776
Oid tupType;
37623777
int32 tupTypmod;

‎src/backend/optimizer/util/clauses.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3188,7 +3188,7 @@ eval_const_expressions_mutator(Node *node,
31883188

31893189
arg = eval_const_expressions_mutator((Node *) ntest->arg,
31903190
context);
3191-
if (arg && IsA(arg, RowExpr))
3191+
if (ntest->argisrow && arg && IsA(arg, RowExpr))
31923192
{
31933193
/*
31943194
* We break ROW(...) IS [NOT] NULL into separate tests on
@@ -3200,8 +3200,6 @@ eval_const_expressions_mutator(Node *node,
32003200
List *newargs = NIL;
32013201
ListCell *l;
32023202

3203-
Assert(ntest->argisrow);
3204-
32053203
foreach(l, rarg->args)
32063204
{
32073205
Node *relem = (Node *) lfirst(l);
@@ -3220,10 +3218,17 @@ eval_const_expressions_mutator(Node *node,
32203218
return makeBoolConst(false, false);
32213219
continue;
32223220
}
3221+
3222+
/*
3223+
* Else, make a scalar (argisrow == false) NullTest
3224+
* for this field. Scalar semantics are required
3225+
* because IS [NOT] NULL doesn't recurse; see comments
3226+
* in ExecEvalNullTest().
3227+
*/
32233228
newntest = makeNode(NullTest);
32243229
newntest->arg = (Expr *) relem;
32253230
newntest->nulltesttype = ntest->nulltesttype;
3226-
newntest->argisrow = type_is_rowtype(exprType(relem));
3231+
newntest->argisrow = false;
32273232
newargs = lappend(newargs, newntest);
32283233
}
32293234
/* If all the inputs were constants, result is TRUE */

‎src/test/regress/expected/rowtypes.out

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,3 +657,57 @@ select row_to_json(r) from (select q2,q1 from tt1 offset 0) r;
657657
{"q2":0,"q1":0}
658658
(3 rows)
659659

660+
--
661+
-- IS [NOT] NULL should not recurse into nested composites (bug #14235)
662+
--
663+
explain (verbose, costs off)
664+
select r, r is null as isnull, r is not null as isnotnull
665+
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
666+
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
667+
QUERY PLAN
668+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
669+
Values Scan on "*VALUES*"
670+
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))
671+
(2 rows)
672+
673+
select r, r is null as isnull, r is not null as isnotnull
674+
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
675+
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
676+
r | isnull | isnotnull
677+
-------------+--------+-----------
678+
(1,"(1,2)") | f | t
679+
(1,"(,)") | f | t
680+
(1,) | f | f
681+
(,"(1,2)") | f | f
682+
(,"(,)") | f | f
683+
(,) | t | f
684+
(6 rows)
685+
686+
explain (verbose, costs off)
687+
with r(a,b) as
688+
(values (1,row(1,2)), (1,row(null,null)), (1,null),
689+
(null,row(1,2)), (null,row(null,null)), (null,null) )
690+
select r, r is null as isnull, r is not null as isnotnull from r;
691+
QUERY PLAN
692+
----------------------------------------------------------
693+
CTE Scan on r
694+
Output: r.*, (r.* IS NULL), (r.* IS NOT NULL)
695+
CTE r
696+
-> Values Scan on "*VALUES*"
697+
Output: "*VALUES*".column1, "*VALUES*".column2
698+
(5 rows)
699+
700+
with r(a,b) as
701+
(values (1,row(1,2)), (1,row(null,null)), (1,null),
702+
(null,row(1,2)), (null,row(null,null)), (null,null) )
703+
select r, r is null as isnull, r is not null as isnotnull from r;
704+
r | isnull | isnotnull
705+
-------------+--------+-----------
706+
(1,"(1,2)") | f | t
707+
(1,"(,)") | f | t
708+
(1,) | f | f
709+
(,"(1,2)") | f | f
710+
(,"(,)") | f | f
711+
(,) | t | f
712+
(6 rows)
713+

‎src/test/regress/sql/rowtypes.sql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,3 +286,27 @@ create temp table tt1 as select * from int8_tbl limit 2;
286286
create temp table tt2 () inherits(tt1);
287287
insert into tt2 values(0,0);
288288
select row_to_json(r) from (select q2,q1 from tt1 offset 0) r;
289+
290+
--
291+
-- IS [NOT] NULL should not recurse into nested composites (bug #14235)
292+
--
293+
294+
explain (verbose, costs off)
295+
select r, r is null as isnull, r is not null as isnotnull
296+
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
297+
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
298+
299+
select r, r is null as isnull, r is not null as isnotnull
300+
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
301+
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
302+
303+
explain (verbose, costs off)
304+
with r(a,b) as
305+
(values (1,row(1,2)), (1,row(null,null)), (1,null),
306+
(null,row(1,2)), (null,row(null,null)), (null,null) )
307+
select r, r is null as isnull, r is not null as isnotnull from r;
308+
309+
with r(a,b) as
310+
(values (1,row(1,2)), (1,row(null,null)), (1,null),
311+
(null,row(1,2)), (null,row(null,null)), (null,null) )
312+
select r, r is null as isnull, r is not null as isnotnull from r;

0 commit comments

Comments
 (0)