Skip to content

Commit 9492cf8

Browse files
tglsfdctglsfdc
committed
Fix assorted fallout from IS [NOT] NULL patch.
Commits 4452000 et al established semantics for NullTest.argisrow that are a bit different from its initial conception: rather than being merely a cache of whether we've determined the input to have composite type, the flag now has the further meaning that we should apply field-by-field testing as per the standard's definition of IS [NOT] NULL. If argisrow is false and yet the input has composite type, the construct instead has the semantics of IS [NOT] DISTINCT FROM NULL. Update the comments in primnodes.h to clarify this, and fix ruleutils.c and deparse.c to print such cases correctly. In the case of ruleutils.c, this merely results in cosmetic changes in EXPLAIN output, since the case can't currently arise in stored rules. However, it represents a live bug for deparse.c, which would formerly have sent a remote query that had semantics different from the local behavior. (From the user's standpoint, this means that testing a remote nested-composite column for null-ness could have had unexpected recursive behavior much like that fixed in 4452000.) In a related but somewhat independent fix, make plancat.c set argisrow to false in all NullTest expressions constructed to represent "attnotnull" constructs. Since attnotnull is actually enforced as a simple null-value check, this is a more accurate representation of the semantics; we were previously overpromising what it meant for composite columns, which might possibly lead to incorrect planner optimizations. (It seems that what the SQL spec expects a NOT NULL constraint to mean is an IS NOT NULL test, so arguably we are violating the spec and should fix attnotnull to do the other thing. If we ever do, this part should get reverted.) Back-patch, same as the previous commit. Discussion: <10682.1469566308@sss.pgh.pa.us>
1 parent 46b773d commit 9492cf8

File tree

5 files changed

+77
-20
lines changed

5 files changed

+77
-20
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2347,10 +2347,27 @@ deparseNullTest(NullTest *node, deparse_expr_cxt *context)
23472347

23482348
appendStringInfoChar(buf, '(');
23492349
deparseExpr(node->arg, context);
2350-
if (node->nulltesttype == IS_NULL)
2351-
appendStringInfoString(buf, " IS NULL)");
2350+
2351+
/*
2352+
* For scalar inputs, we prefer to print as IS [NOT] NULL, which is
2353+
* shorter and traditional. If it's a rowtype input but we're applying a
2354+
* scalar test, must print IS [NOT] DISTINCT FROM NULL to be semantically
2355+
* correct.
2356+
*/
2357+
if (node->argisrow || !type_is_rowtype(exprType((Node *) node->arg)))
2358+
{
2359+
if (node->nulltesttype == IS_NULL)
2360+
appendStringInfoString(buf, " IS NULL)");
2361+
else
2362+
appendStringInfoString(buf, " IS NOT NULL)");
2363+
}
23522364
else
2353-
appendStringInfoString(buf, " IS NOT NULL)");
2365+
{
2366+
if (node->nulltesttype == IS_NULL)
2367+
appendStringInfoString(buf, " IS NOT DISTINCT FROM NULL)");
2368+
else
2369+
appendStringInfoString(buf, " IS DISTINCT FROM NULL)");
2370+
}
23542371
}
23552372

23562373
/*

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1198,7 +1198,13 @@ get_relation_constraints(PlannerInfo *root,
11981198
att->attcollation,
11991199
0);
12001200
ntest->nulltesttype = IS_NOT_NULL;
1201-
ntest->argisrow = type_is_rowtype(att->atttypid);
1201+
1202+
/*
1203+
* argisrow=false is correct even for a composite column,
1204+
* because attnotnull does not represent a SQL-spec IS NOT
1205+
* NULL test in such a case, just IS DISTINCT FROM NULL.
1206+
*/
1207+
ntest->argisrow = false;
12021208
ntest->location = -1;
12031209
result = lappend(result, ntest);
12041210
}

‎src/backend/utils/adt/ruleutils.c

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8047,17 +8047,43 @@ get_rule_expr(Node *node, deparse_context *context,
80478047
if (!PRETTY_PAREN(context))
80488048
appendStringInfoChar(buf, '(');
80498049
get_rule_expr_paren((Node *) ntest->arg, context, true, node);
8050-
switch (ntest->nulltesttype)
8050+
8051+
/*
8052+
* For scalar inputs, we prefer to print as IS [NOT] NULL,
8053+
* which is shorter and traditional. If it's a rowtype input
8054+
* but we're applying a scalar test, must print IS [NOT]
8055+
* DISTINCT FROM NULL to be semantically correct.
8056+
*/
8057+
if (ntest->argisrow ||
8058+
!type_is_rowtype(exprType((Node *) ntest->arg)))
80518059
{
8052-
case IS_NULL:
8053-
appendStringInfoString(buf, " IS NULL");
8054-
break;
8055-
case IS_NOT_NULL:
8056-
appendStringInfoString(buf, " IS NOT NULL");
8057-
break;
8058-
default:
8059-
elog(ERROR, "unrecognized nulltesttype: %d",
8060-
(int) ntest->nulltesttype);
8060+
switch (ntest->nulltesttype)
8061+
{
8062+
case IS_NULL:
8063+
appendStringInfoString(buf, " IS NULL");
8064+
break;
8065+
case IS_NOT_NULL:
8066+
appendStringInfoString(buf, " IS NOT NULL");
8067+
break;
8068+
default:
8069+
elog(ERROR, "unrecognized nulltesttype: %d",
8070+
(int) ntest->nulltesttype);
8071+
}
8072+
}
8073+
else
8074+
{
8075+
switch (ntest->nulltesttype)
8076+
{
8077+
case IS_NULL:
8078+
appendStringInfoString(buf, " IS NOT DISTINCT FROM NULL");
8079+
break;
8080+
case IS_NOT_NULL:
8081+
appendStringInfoString(buf, " IS DISTINCT FROM NULL");
8082+
break;
8083+
default:
8084+
elog(ERROR, "unrecognized nulltesttype: %d",
8085+
(int) ntest->nulltesttype);
8086+
}
80618087
}
80628088
if (!PRETTY_PAREN(context))
80638089
appendStringInfoChar(buf, ')');

‎src/include/nodes/primnodes.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,8 +1099,16 @@ typedef struct XmlExpr
10991099
* NullTest represents the operation of testing a value for NULLness.
11001100
* The appropriate test is performed and returned as a boolean Datum.
11011101
*
1102-
* NOTE: the semantics of this for rowtype inputs are noticeably different
1103-
* from the scalar case. We provide an "argisrow" flag to reflect that.
1102+
* When argisrow is false, this simply represents a test for the null value.
1103+
*
1104+
* When argisrow is true, the input expression must yield a rowtype, and
1105+
* the node implements "row IS [NOT] NULL" per the SQL standard. This
1106+
* includes checking individual fields for NULLness when the row datum
1107+
* itself isn't NULL.
1108+
*
1109+
* NOTE: the combination of a rowtype input and argisrow==false does NOT
1110+
* correspond to the SQL notation "row IS [NOT] NULL"; instead, this case
1111+
* represents the SQL notation "row IS [NOT] DISTINCT FROM NULL".
11041112
* ----------------
11051113
*/
11061114

@@ -1114,7 +1122,7 @@ typedef struct NullTest
11141122
Expr xpr;
11151123
Expr *arg; /* input expression */
11161124
NullTestType nulltesttype; /* IS NULL, IS NOT NULL */
1117-
bool argisrow; /* T if input is of a composite type */
1125+
bool argisrow; /* T to perform field-by-field null checks */
11181126
int location; /* token location, or -1 if unknown */
11191127
} NullTest;
11201128

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -664,10 +664,10 @@ explain (verbose, costs off)
664664
select r, r is null as isnull, r is not null as isnotnull
665665
from (values (1,row(1,2)), (1,row(null,null)), (1,null),
666666
(null,row(1,2)), (null,row(null,null)), (null,null) ) r(a,b);
667-
QUERY PLAN
668-
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
667+
QUERY PLAN
668+
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
669669
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))
670+
Output: ROW("*VALUES*".column1, "*VALUES*".column2), (("*VALUES*".column1 IS NULL) AND ("*VALUES*".column2 IS NOT DISTINCT FROM NULL)), (("*VALUES*".column1 IS NOT NULL) AND ("*VALUES*".column2 IS DISTINCT FROM NULL))
671671
(2 rows)
672672

673673
select r, r is null as isnull, r is not null as isnotnull

0 commit comments

Comments
 (0)