diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2016-07-26 15:25:02 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2016-07-26 15:25:02 -0400 |
commit | d243bf77c2d3888fb263c55317c2453cb437d1de (patch) | |
tree | 6ec5799f24acb9168f1f73ccc7da84fd87cb662d /src | |
parent | b1fa6c0eb9fadc9eada014227a67eadfec936960 (diff) |
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>
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/executor/execQual.c | 15 | ||||
-rw-r--r-- | src/backend/optimizer/util/clauses.c | 12 | ||||
-rw-r--r-- | src/test/regress/expected/rowtypes.out | 54 | ||||
-rw-r--r-- | src/test/regress/sql/rowtypes.sql | 24 |
4 files changed, 101 insertions, 4 deletions
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index ebc6a3f1500..ea8850e12cf 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -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; diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index e7bfc38d566..667e682e2d1 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -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 */ diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out index 96da4be048c..93301992c27 100644 --- a/src/test/regress/expected/rowtypes.out +++ b/src/test/regress/expected/rowtypes.out @@ -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) + diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql index 69851ec2ff1..efe94f412cd 100644 --- a/src/test/regress/sql/rowtypes.sql +++ b/src/test/regress/sql/rowtypes.sql @@ -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; |