8000 Fix constant-folding of ROW(...) IS [NOT] NULL with composite fields. · rtpg/postgres@d243bf7 · GitHub
[go: up one dir, main page]
Skip to content

Commit d243bf7

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 b1fa6c0 commit d243bf7

File tree

5 files changed

+135
-40
lines changed

5 files changed

+135
-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. When this behavior is not suitable,
368-
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
@@ -3701,6 +3701,21 @@ ExecEvalNullTest(NullTestState *nstate,
37013701

37023702
if (ntest->argisrow && !(*isNull))
37033703
{
3704+
/*
3705+
* The SQL standard defines IS [NOT] NULL for a non-null rowtype
3706+
* argument as:
3707+
*
3708+
* "R IS NULL" is true if every field is the null value.
3709+
*
3710+
* "R IS NOT NULL" is true if no field is the null value.
3711+
*
3712+
* This definition is (apparently intentionally) not recursive; so our
3713+
* tests on the fields are primitive attisnull tests, not recursive
3714+
* checks to see if they are all-nulls or no-nulls rowtypes.
3715+
*
3716+
* The standard does not consider the possibility of zero-field rows,
3717+
* but here we consider them to vacuously satisfy both predicates.
3718+
*/
37043719
HeapTupleHeader tuple;
37053720
Oid tupType;
37063721
int32 tupTypmod;

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2970,7 +2970,7 @@ eval_const_expressions_mutator(Node *node,
29702970

29712971
arg = eval_const_expressions_mutator((Node *) ntest->arg,
29722972
context);
2973-
if (arg && IsA(arg, RowExpr))
2973+
if (ntest->argisrow && arg && IsA(arg, RowExpr))
29742974
{
29752975
/*
29762976
* We break ROW(...) IS [NOT] NULL into separate tests on its
@@ -2981,8 +2981,6 @@ eval_const_expressions_mutator(Node *node,
29812981
List *newargs = NIL;
29822982
ListCell *l;
29832983

2984-
Assert(ntest->argisrow);
2985-
29862984
foreach(l, rarg->args)
29872985
{
29882986
Node *relem = (Node *) lfirst(l);
@@ -3001,10 +2999,16 @@ eval_const_expressions_mutator(Node *node,
30012999
return makeBoolConst(false, false);
30023000
continue;
30033001
}
3002+
3003+
/*
3004+
* Else, make a scalar (argisrow == false) NullTest for this
3005+
* field. Scalar semantics are required because IS [NOT] NULL
3006+
* doesn't recurse; see comments in ExecEvalNullTest().
3007+
*/
30043008
newntest = makeNode(NullTest);
30053009
newntest->arg = (Expr *) relem;
30063010
newntest->nulltesttype = ntest->nulltesttype;
3007-
newntest->argisrow = type_is_rowtype(exprType(relem));
3011+
newntest->argisrow = false;
30083012
newargs = lappend(newargs, newntest);
30093013
}
30103014
/* 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
@@ -442,3 +442,57 @@ select (row('Jim', 'Beam')).text; -- error
442442
ERROR: could not identify column "text" in record data type
443443
LINE 1: select (row('Jim', 'Beam')).text;
444444
^
445+
--
446+
-- IS [NOT] NULL should not recurse into nested composites (bug #14235)
447+
--
448+
explain (verbose, costs off)
449+
select r, r is null as isnull, r is not null as isnotnull
450+
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
451+
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
452+
QUERY PLAN
453+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
454+
Values Scan on "*VALUES*"
455+
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))
456+
(2 rows)
457+
458+
select r, r is null as isnull, r is not null as isnotnull
459+
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
460+
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
461+
r | isnull | isnotnull
462+
-------------+--------+-----------
463+
(1,"(1,2)") | f | t
464+
(1,"(,)") | f | t
465+
(1,) | f | f
466+
(,"(1,2)") | f | f
467+
(,"(,)") | f | f
468+
(,) | t | f
469+
(6 rows)
470+
471+
explain (verbose, costs off)
472+
with r(a,b) as
473+
(values (1,row(1,2)), (1,row(null,null)), (1,null),
474+
(null,row(1,2)), (null,row(null,null)), (null,null) )
475+
select r, r is null as isnull, r is not null as isnotnull from r;
476+
QUERY PLAN
477+
----------------------------------------------------------
478+
CTE Scan on r
479+
Output: r.*, (r.* IS NULL), (r.* IS NOT NULL)
480+
CTE r
481+
-> Values Scan on "*VALUES*"
482+
Output: "*VALUES*".column1, "*VALUES*".column2
483+
(5 rows)
484+
485+
with r(a,b) as
486+
(values (1,row(1,2)), (1,row(null,null)), (1,null),
487+
(null,row(1,2)), (null,row(null,null)), (null,null) )
488+
select r, r is null as isnull, r is not null as isnotnull from r;
489+
r | isnull | isnotnull
490+
-------------+--------+-----------
491+
(1,"(1,2)") | f | t
492+
(1,"(,)") | f | t
493+
(1,) | f | f
494+
(,"(1,2)") | f | f
495+
(,"(,)") | f | f
496+
(,) | t | f
497+
(6 rows)
498+

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,3 +209,27 @@ select cast (row('Jim', 'Beam') as text);
209209
select (row('Jim', 'Beam'))::text;
210210
select text(row('Jim', 'Beam')); -- error
211211
select (row('Jim', 'Beam')).text; -- error
212+
213+
--
214+
-- IS [NOT] NULL should not recurse into nested composites (bug #14235)
215+
--
216+
217+
explain (verbose, costs off)
218+
select r, r is null as isnull, r is not null as isnotnull
219+
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
220+
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
221+
222+
select r, r is null as isnull, r is not null as isnotnull
223+
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
224+
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
225+
226+
explain (verbose, costs off)
227+
with r(a,b) as
228+
(values (1,row(1,2)), (1,row(null,null)), (1,null),
229+
(null,row(1,2)), (null,row(null,null)), (null,null) )
230+
select r, r is null as isnull, r is not null as isnotnull from r;
231+
232+
with r(a,b) as
233+
(values (1,row(1,2)), (1,row(null,null)), (1,null),
234+
(null,row(1,2)), (null,row(null,null)), (null,null) )
235+
select r, r is null as isnull, r is not null as isnotnull from r;

0 commit comments

Comments
 (0)
0