Re: Bug with plpgsql handling of NULL argument of compound type

On Fri, Jul 22, 2016 at 3:04 PM, Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:

> On Fri, Jul 22, 2016 at 1:39 PM, David G. Johnston
> <david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
> > On Fri, Jul 22, 2016 at 2:13 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>
> >> There is a rather squishy question as to whether NULL::composite_type
> >> should be semantically equivalent to ROW(NULL,NULL,...)::composite_type.
> >> If it is, then the SELECT should have failed before even getting into
> the
> >> plpgsql function, because ROW(NULL,NULL) is surely not a valid value of
> >> type c. The SQL standard seems to believe that these things *are*
> >> equivalent (at least, that was how we read the spec for IS [NOT] NULL).
> >>
> >
> > I dislike that they are considered equal in various circumstances but if
> > that's we are guided toward c'est la vie.
>
> Not sure we are guided there. Currently we follow the spec
> specifically with the IS NULL operator but not in other cases. For
> example.
> postgres=# select row(null, null) is null;
> ?column?
> ──────────
> t
>

​[...]

> ​
>
> The basic problem we have is that in postgres the record variable is a
> distinct thing from its contents and the spec does not treat it that
> was. For my part, I think the spec is totally out to lunch on this
> point but we've been stuck with the status quo for quite some time now
> -- there's been no compelling narrative that suggests how things
> should be changed and to what.
>

​In short,

1) We should discourage/remove the NOT NULL aspect of DOMAINs.

2) If one wishes to implement composite types defining not null components
it should
2a) be done on the CREATE TYPE statement
2b) involve behind-the-scenes transformation of row(null, null)::ctype to
null::ctype and null::ctype should not be validated, ever

​I cannot speak to the standard nor the entirety of our implementation in
this area, but...​

​I don't personally have a problem with (conceptually, not actual
evaluations):

select row(null, null) is null --> true
select null is null --> true
select null = row(null, null) --> false (at least with respect to
implementation)

IS NULL and equality are two different things. That both constructs
evaluate to null but are not implementation equivalent, while maybe a bit
ugly, doesn't offend me. I'd just consider "row(null, null) is null" to be
a special case circumstance required by the spec and move on.

Then, forcing "null::composite" to be evaluated like "row(null,
null)::composite" ​can be considered incorrect.

​If anything, ROW(null, null)::ctype should hard transformed to
"null::ctype" but not the other way around. Only after an attempt to
transform row(null, null) is performed should the type constraints be
applied to those values not able to be transformed.

That all said I'm still disinclined to suggest/allow people to add NOT NULL
constraints to DOMAINs. All other types in the system are basically
validated using the rule: "if there is a non-null value of this type ensure
that said value conforms". As domains are types they should conform to
this policy. A composite type is a container for other types. The
container itself should be allowed to have its own rules - in much the same
way as a table does [1].

My concern regarding the above is that the row/isnull behavior is all
defined around the composite type yet the notnull constraint is attached to
the DOMAIN and I dislike that disconnect. Having the NOT NULL on the
composite type and only having it applied after any value of the form
row(null, null)::ctype is reduced to null::ctype - a form in which all
subfield constraints are ignored - would be closer to my liking. It also
avoids other problems related to DOMAINs but not requiring their use.

David J.

[1] I see a problem here if one were to change the behavior of explicit
composite types. w.r.t. tables the NOT NULL constraints is not part of the
implicitly created type but if we allow an explicitly declared composite to
use NOT NULL and don't default the implicit types the disconnect could be
confusing.