diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2016-07-28 16:09:15 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2016-07-28 16:09:15 -0400 |
commit | 1e2f96f0a56c2d67a84ffb58383e6354546cf96f (patch) | |
tree | 8d1c0063defe100f184461b627bb5dde8f07fb48 /src/backend/utils/adt/ruleutils.c | |
parent | 884aec4f8505d9b4c767cf0f2456e12b843688cc (diff) | |
download | postgresql-1e2f96f0a56c2d67a84ffb58383e6354546cf96f.tar.gz postgresql-1e2f96f0a56c2d67a84ffb58383e6354546cf96f.zip |
Fix assorted fallout from IS [NOT] NULL patch.
Commits 4452000f3 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 4452000f3.)
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>
Diffstat (limited to 'src/backend/utils/adt/ruleutils.c')
-rw-r--r-- | src/backend/utils/adt/ruleutils.c | 46 |
1 files changed, 36 insertions, 10 deletions
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 97bdccf6ec6..8a6d0ad9661 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -7890,17 +7890,43 @@ get_rule_expr(Node *node, deparse_context *context, if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, '('); get_rule_expr_paren((Node *) ntest->arg, context, true, node); - switch (ntest->nulltesttype) + + /* + * For scalar inputs, we prefer to print as IS [NOT] NULL, + * which is shorter and traditional. If it's a rowtype input + * but we're applying a scalar test, must print IS [NOT] + * DISTINCT FROM NULL to be semantically correct. + */ + if (ntest->argisrow || + !type_is_rowtype(exprType((Node *) ntest->arg))) { - case IS_NULL: - appendStringInfoString(buf, " IS NULL"); - break; - case IS_NOT_NULL: - appendStringInfoString(buf, " IS NOT NULL"); - break; - default: - elog(ERROR, "unrecognized nulltesttype: %d", - (int) ntest->nulltesttype); + switch (ntest->nulltesttype) + { + case IS_NULL: + appendStringInfoString(buf, " IS NULL"); + break; + case IS_NOT_NULL: + appendStringInfoString(buf, " IS NOT NULL"); + break; + default: + elog(ERROR, "unrecognized nulltesttype: %d", + (int) ntest->nulltesttype); + } + } + else + { + switch (ntest->nulltesttype) + { + case IS_NULL: + appendStringInfoString(buf, " IS NOT DISTINCT FROM NULL"); + break; + case IS_NOT_NULL: + appendStringInfoString(buf, " IS DISTINCT FROM NULL"); + break; + default: + elog(ERROR, "unrecognized nulltesttype: %d", + (int) ntest->nulltesttype); + } } if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, ')'); |