Re: BUG #14235: inconsistencies with IS NULL / IS NOT NULL - Mailing list pgsql-bugs

From Andrew Gierth
Subject Re: BUG #14235: inconsistencies with IS NULL / IS NOT NULL
Date
Msg-id 8760sg96u2.fsf@news-spur.riddles.org.uk
Whole thread Raw
In response to BUG #14235: inconsistencies with IS NULL / IS NOT NULL  (andrew@tao11.riddles.org.uk)
Responses Re: BUG #14235: inconsistencies with IS NULL / IS NOT NULL
Re: BUG #14235: inconsistencies with IS NULL / IS NOT NULL
Re: BUG #14235: inconsistencies with IS NULL / IS NOT NULL
List pgsql-bugs
>>>>> "andrew" == andrew  <andrew@tao11.riddles.org.uk> writes:

 andrew> While discussing the issues resulting from the spec's
 andrew> definition of IS NULL and IS NOT NULL on composite values, we
 andrew> discovered the following inconsistent cases, all of which
 andrew> appear to be the fault of eval_const_expressions.

 andrew> In short, the transformations applied to NullTest nodes change
 andrew> the semantics, which can then make the final result depend on,
 andrew> for example, whether function calls were inlined or not.

And here's my analysis of what seems to be going on:

The executor, when doing IS [NOT] NULL on a composite value, looks at
each column to see if it is the null value. It does NOT recurse into
nested composite values, and my reading of the spec suggests that this
is correct.

But eval_const_expressions thinks it has license to change

  ROW(a,b) IS NULL

into

  (a IS NULL) AND (b IS NULL)

which has the effect of recursing one level into a nested composite
value.

It seems possible that this could be fixed by simply setting
argisrow=false for all the null tests generated in such cases.
Specifically I've tried this, which seems to fix all the inconsistent
cases:

--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -3308,7 +3308,13 @@ eval_const_expressions_mutator(Node *node,
                                                newntest = makeNode(NullTest);
                                                newntest->arg = (Expr *) relem;
                                                newntest->nulltesttype = ntest->nulltesttype;
-                                               newntest->argisrow = type_is_rowtype(exprType(relem));
+                                               /*
+                                                * We must unconditionally set argisrow false here.
+                                                * Otherwise, we'd be treating a nested composite
+                                                * structure as though it were at top level, which
+                                                * would change the semantics of the test.
+                                                */
+                                               newntest->argisrow = false;
                                                newntest->location = ntest->location;
                                                newargs = lappend(newargs, newntest);
                                        }


--
Andrew (irc:RhodiumToad)

pgsql-bugs by date:

Previous
From: andrew@tao11.riddles.org.uk
Date:
Subject: BUG #14235: inconsistencies with IS NULL / IS NOT NULL
Next
From: Andrew Gierth
Date:
Subject: Re: BUG #14235: inconsistencies with IS NULL / IS NOT NULL