Re: Proposal: revert behavior of IS NULL on row types

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 7/22/16 7:01 PM, Andrew Gierth wrote:
>> In light of the fact that it is an endless cause of bugs both in pg and
>> potentially to applications, I propose that we cease attempting to
>> conform to the spec's definition of IS NULL in favour of the following
>> rules:

> I can't see how we would incompatibly change existing
> standards-conforming behavior merely because users are occasionally
> confused and there are some bugs in corner cases.

It seems clear that Andrew's proposal to reject the spec's definition that
ROW(NULL,NULL,...) IS NULL is true has failed.  So ExecEvalNullTest()
should keep on doing what it's doing.  There are several related issues
however:

1. As per bug #14235, eval_const_expressions() behaves differently from
ExecEvalNullTest() for nested-composite-types cases.  It is inarguably
a bug that they don't give the same answers.  So far, no one has spoken
against the conclusion reached in that thread that ExecEvalNullTest()
correctly implements the spec's semantics and eval_const_expressions()
does not.  Therefore I propose to apply and back-patch Andrew's fix from
that thread.

2. Our (or at least my) previous understanding was that ROW(NULL,NULL,...)
ought to be considered NULL for all purposes, and that our failure to
do so anywhere except ExecEvalNullTest was a spec violation we should do
something about someday.  But the bug #14235 thread makes it clear that
that's not so, and that only in very limited cases is there an argument
for applying IS [NOT] NULL's behavior to other constructs.  COALESCE()
was mentioned as something that maybe should change.  I'm inclined to vote
"no, let's keep COALESCE as-is".  The spec's use of, essentially, macro
expansion to define COALESCE is just stupid, not least because it's
impossible to specify the expected at-most-once evaluation of each
argument expression that way.  (They appear to try to dodge that question
by forbidding argument expressions with side-effects, which is just lame.)
But had they written out a definition of COALESCE's behavior in words,
they would almost certainly have written "V is not the null value" not
"V IS NOT NULL".  Anyone who stopped to think about it would surely think
that the result of COALESCE(ROW(1,NULL), NULL) should not be NULL.  So I
think the apparent mandate to use IS NOT NULL's semantics for rowtype
values in COALESCE is just an artifact of careless drafting.  Between that
and the backwards-compatibility hazards of changing, it's not worth it.

3. Andrew also revived the bug #7808 thread in which it was complained
that ExecMakeTableFunctionResult should not fail on null results from
functions returning SETOF composite.  That's related only in that the
proposed fix is to translate a plain NULL into ROW(NULL,NULL,...).
I do not much like the implementation details of his proposed patch,
but I think making that translation is probably better than failing
altogether.  Given the infrequency of field complaints, my recommendation
here is to fix it in HEAD but not back-patch.

Comments?
        regards, tom lane