Re: [BUGS] BUG #5053: domain constraints still leak

The following bug has been logged online:

Bug reference: 5053
Logged by: Andrew Gierth
Email address: andrew(at)tao11(dot)riddles(dot)org(dot)uk
PostgreSQL version: 8.5devel
Operating system: FreeBSD
Description: domain constraints still leak
Details:

Domain NOT NULL constraints (and probably other constraints too) aren't
being enforced in some code paths. e.g.

\pset null '<NULL>'
create domain tstdom as integer not null;
create table test (a tstdom);
insert into test values (null);
ERROR: domain tstdom does not allow null values

all correct up to now, but:

insert into test select (r).* from (select null::test as r) s;
INSERT 0 1

oops.

select * from test;
a
--------
<NULL>
(1 row)

"Andrew Gierth" <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> Domain NOT NULL constraints (and probably other constraints too) aren't
> being enforced in some code paths. e.g.

The example you give seems to tie really closely into the debate about
whether a composite null is identically the same thing as ROW(NULL,NULL)
or not. In short, we have

regression=# create domain tstdom as integer not null;
CREATE DOMAIN
regression=# create table test (a tstdom);
CREATE TABLE
regression=# select null::test;
test
------

(1 row)

regression=# select row(null)::test;
ERROR: domain tstdom does not allow null values

It's possible to argue that in the first form, there isn't any tstdom
column there at all, so no constraint violation. So I guess this is
a case that we need to think about while debating the what-is-a-null
question.

regards, tom lane

PS: of course, domain not null constraints are horribly broken and
impossible to make behave sanely anyhow ...

On Sun, Sep 13, 2009 at 10:28 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Andrew Gierth" <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
>> Domain NOT NULL constraints (and probably other constraints too) aren't
>> being enforced in some code paths. e.g.
>
> The example you give seems to tie really closely into the debate about
> whether a composite null is identically the same thing as ROW(NULL,NULL)
> or not.  In short, we have

It seems like regardless of this discussion you oughtn't to be able to
store a NULL into that table column. But maybe I'm just confused.

...Robert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> It seems like regardless of this discussion you oughtn't to be able to
> store a NULL into that table column. But maybe I'm just confused.

The system is relying on the not-unreasonable assumption that if it
extracts a column of type X from someplace, what it has is a valid value
of type X. Depending on what we decide about the whole composite-null
mess, maybe we will be forced to abandon that assumption ... but I'm
sure not going to do so until my back is to the wall.

regards, tom lane

On Mon, Sep 14, 2009 at 10:22:34AM -0400, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > It seems like regardless of this discussion you oughtn't to be able to
> > store a NULL into that table column. But maybe I'm just confused.
>
> The system is relying on the not-unreasonable assumption that if it
> extracts a column of type X from someplace, what it has is a valid value
> of type X.

Yup; the deeper problem seems to be that the table was created as:

create table test (a tstdom);

and not as:

create table test (a tstdom not null);

which is how you seem to be treating it.

> Depending on what we decide about the whole composite-null
> mess, maybe we will be forced to abandon that assumption ... but I'm
> sure not going to do so until my back is to the wall.

There seems to be a little space yet!

This whole issue seems only distantly related to the treatment of null
rows to me. I think PG has got its semantics confused along the way
somewhere and things need tweaking. The only way I can get it all to
work nicely in my head is if ROW(NULL) evaluates to a NULL value (and
not a row containing a NULL value, as it does at the moment) and the
NULL/NOT NULL constraint on the CREATE DOMAIN is used somehow for the
nullness constraint of any columns using this domain. It's the second
part that seems to be more critical, but there are various ways of
interpreting the meaning.

I'm tempted to say that the nullness specified in the domain puts a
bound on the amount of nullness available--i.e. it would be impossible
to create a nullable column from a domain that specified NOT NULL. The
reason it's only a "limit" is that it seems useful to be able to say
that a normally nullable domain can't be null for this column in this
table. Not sure if this is what people want though.

You then get into fun cases like:

create domain tstdom as integer;
create domain tstdom2 as tstdom;

--
Sam http://samason.me.uk/

On Mon, Sep 14, 2009 at 10:22 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> It seems like regardless of this discussion you oughtn't to be able to
>> store a NULL into that table column.  But maybe I'm just confused.
>
> The system is relying on the not-unreasonable assumption that if it
> extracts a column of type X from someplace, what it has is a valid value
> of type X.  Depending on what we decide about the whole composite-null
> mess, maybe we will be forced to abandon that assumption ... but I'm
> sure not going to do so until my back is to the wall.

I haven't read the code in this area, but for what it's worth, I guess
I lean toward the view that treating a row of NULLs as being the same
thing as an undecorated NULL does not make very much sense. If I have
a table row which contains (1, NULL, NULL) and I update the first
column to be NULL, I feel like I now have (NULL, NULL, NULL), not just
NULL. Every other programming language I'm aware of makes this
distinction - for good reasons - and I don't really see any reason why
SQL should do anything different.

Under that view, null::test is not itself a test, but denotes the
absence of one. Trying to store this value in a table can either fail
outright (on the theory that you can't store the absence of something
in a table), or else we can - as a special case - treat assignment
from null to an actual object as a request to assign null to each
column (which will fail if there exists a column into which a NULL of
the associated column type can't be stored).

...Robert

On Mon, Sep 14, 2009 at 11:16:23AM -0400, Robert Haas wrote:
> I haven't read the code in this area, but for what it's worth, I guess
> I lean toward the view that treating a row of NULLs as being the same
> thing as an undecorated NULL does not make very much sense.

I agree; when compared to most languages it doesn't. When compared
to the semantics of the other operators in SQL it gets better. I
personally think PG should strive to be internally consistent rather
than consistency with other (non-SQL based) languages.

> If I have
> a table row which contains (1, NULL, NULL) and I update the first
> column to be NULL, I feel like I now have (NULL, NULL, NULL), not just
> NULL. Every other programming language I'm aware of makes this
> distinction - for good reasons - and I don't really see any reason why
> SQL should do anything different.

I'm not aware of any other language that does the automatic "lifting"
(to borrow nomenclature from Haskell) that SQL does, allowing NULL
appear in *every* type. Java, for example, has null references,
but these are very different creatures from nulls in databases--the
programmer has to explicitly deal with them all the time and also they
only apply to references. Taken another way, each object in a normal
imperative language has its own identity, but in a database two rows
that "look" the same are the same. Thirdly, IS NULL is defined to look
"inside" composite values to see if they're "really" null. Its these
differences in semantics that seem to make it all OK.

> Under that view, null::test is not itself a test, but denotes the
> absence of one.

OK, but how can you distinguish NULL from ROW(NULL,NULL)?

SELECT v IS NULL, v.a, v.b
FROM (SELECT NULL, NULL) v(a,b);

Would appear to return the same thing if ROW(NULL,NULL) evaluated to
NULL or not. The only time it would show up is when you're trying to
save the value into a table and I think this would tend to do the right
thing more often. For example:

INSERT INTO t (id,rv)
SELECT f.id, b
FROM foo f
LEFT JOIN bar b ON (f.id = b.id);

Would fail if any bar's didn't exist, whereas the current behavior is
to insert a row with rv containing all null values. You can't test for
this case because IS NULL would return the "wrong" thing as it looks
inside composites.

--
Sam http://samason.me.uk/

Sam Mason <sam(at)samason(dot)me(dot)uk> wrote:

> the deeper problem seems to be that the table was created as:
>
> create table test (a tstdom);
>
> and not as:
>
> create table test (a tstdom not null);

Given that tstdom is declared as NOT NULL, is this difference
considered a *feature* or is it an implementation quirk?

-Kevin

On Mon, Sep 14, 2009 at 10:54:07AM -0500, Kevin Grittner wrote:
> Sam Mason <sam(at)samason(dot)me(dot)uk> wrote:
> > the deeper problem seems to be that the table was created as:
> >
> > create table test (a tstdom);
> >
> > and not as:
> >
> > create table test (a tstdom not null);
>
> Given that tstdom is declared as NOT NULL, is this difference
> considered a *feature* or is it an implementation quirk?

That's why I pointed it out!

Based on my reading of the SQL spec (and reading about Codd's
descriptions of domains) I'd say it was a bug/implementation quirk.

--
Sam http://samason.me.uk/

[ probably time to move this thread to -hackers ]

There is some moderately interesting reading material in section
4.17.4 "Domain constraints" of SQL:2008. In particular, it appears to
me that the standard goes out of its way to NOT claim that every value
that "is of" a domain type satisfies the domain's constraints. It looks
to me that the implementation they have in mind is that domain
constraints are to be checked:

(1) when a value is assigned to a *table* column having that domain type;

(2) when a value is converted to that domain type by an *explicit*
cast construct;

(3) nowhere else.

If I'm reading this right, it sidesteps most of the concerns we have
been worrying about here, at the cost of being perhaps more surprising
and less useful than one would expect. It would also mean that a lot
of our existing domain behavior is wrong. I think there is ammunition
here for an argument that, in effect, values "in flight" in expression
or query evaluation should always be considered to be of base types,
and domain constraints should only be checked when assigning to a
persistent storage location such as a table field or plpgsql variable
(plus the special case for CAST constructs).

regards, tom lane

>>>>> "Tom" == Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

Tom> [ probably time to move this thread to -hackers ]

Tom> There is some moderately interesting reading material in section
Tom> 4.17.4 "Domain constraints" of SQL:2008. In particular, it
Tom> appears to me that the standard goes out of its way to NOT claim
Tom> that every value that "is of" a domain type satisfies the
Tom> domain's constraints. It looks to me that the implementation
Tom> they have in mind is that domain constraints are to be checked:

Tom> (1) when a value is assigned to a *table* column having that
Tom> domain type;

Tom> (2) when a value is converted to that domain type by an
Tom> *explicit* cast construct;

Tom> (3) nowhere else.

By my reading it's a bit more involved than that. In particular, if
you cast from one rowtype to another, that seems to be defined in terms
of individual casts of each column, so CAST(ROW(null) TO rowtype) where
rowtype has one column of a not-null domain type would still count as an
explicit cast to the domain.

But there's a kicker: in Subclause 6.12, <cast specification>, in the
General Rules is:

2) Case:
a) If the <cast operand> specifies NULL, then the result of CS is
the null value and no further General Rules of this Subclause
are applied.
b) If the <cast operand> specifies an <empty specification>, then
the result of CS is an empty collection of declared type TD
and no further General Rules of this Subclause are applied.
c) If SV is the null value, then the result of CS is the null
value and no further General Rules of this Subclause are
applied.

That "no further General Rules" clause implies (assuming it's not a
blatant mistake in the spec) that this rule is therefore skipped in
the case of nulls:

21) If the <cast specification> contains a <domain name> and that
<domain name> refers to a domain that contains a <domain
constraint> and if TV does not satisfy the <check constraint
definition> simply contained in the <domain constraint>, then an
exception condition is raised: integrity constraint violation.

Which would imply that you can cast a NULL to a domain type even if
that would violate a constraint. Which would pretty much leave
actual assignment to storage as being the only place for the check
to happen.

--
Andrew (irc:RhodiumToad)

On Mon, Sep 14, 2009 at 11:20:59PM -0400, Tom Lane wrote:
> There is some moderately interesting reading material in section
> 4.17.4 "Domain constraints" of SQL:2008.

Not sure where to look for a copy of that, nor any particularly helpful
links :(

> In particular, it appears to
> me that the standard goes out of its way to NOT claim that every value
> that "is of" a domain type satisfies the domain's constraints. It looks
> to me that the implementation they have in mind is that domain
> constraints are to be checked:
>
> (1) when a value is assigned to a *table* column having that domain type;
>
> (2) when a value is converted to that domain type by an *explicit*
> cast construct;
>
> (3) nowhere else.

I struggle to get any useful meaning out of the SQL specs, but that
sounds about right to me.

> If I'm reading this right, it sidesteps most of the concerns we have
> been worrying about here, at the cost of being perhaps more surprising
> and less useful than one would expect.

It means that domains are a world away from ADTs (abstract data types)
and just seem to function as quick "templates" for creating new columns.
PG seems to be treating domains as ADTs at the moment, which is the
abstraction that's proved to be more useful in larger programming
projects.

> It would also mean that a lot
> of our existing domain behavior is wrong. I think there is ammunition
> here for an argument that, in effect, values "in flight" in expression
> or query evaluation should always be considered to be of base types,
> and domain constraints should only be checked when assigning to a
> persistent storage location such as a table field or plpgsql variable
> (plus the special case for CAST constructs).

Are you considering changing PGs behavior here? and if so, what would
happen to existing behavior?

--
Sam http://samason.me.uk/

On Tue, Sep 15, 2009 at 05:13:21AM +0100, Andrew Gierth wrote:
> But there's a kicker: in Subclause 6.12, <cast specification>, in the
> General Rules is:
>
> a) If the <cast operand> specifies NULL, then the result of CS is
> the null value and no further General Rules of this Subclause
> are applied.
>
> That "no further General Rules" clause implies (assuming it's not a
> blatant mistake in the spec) that this rule is therefore skipped in
> the case of nulls:

I think the NOT NULL constraint is a PG specific constraint, I can't see
how it's allowed in the spec. Then again, I have trouble parsing the
spec so could well be wrong about this.

The NOT NULL constraint feels wrong as well, what are the semantics of:

CREATE DOMAIN d AS INTEGER NOT NULL;
SELECT a.n AS aa, b.n AS bb
FROM (VALUES (CAST(1 AS d)),(2)) a(n)
LEFT JOIN (VALUES (CAST(1 AS d))) b(n) ON a.n = b.n;

in the presence of it? I'm expecting aa and bb both to come out as
domain "d", but this shouldn't work with what you're saying the current
semantics should be.

--
Sam http://samason.me.uk/

>>>>> "Sam" == Sam Mason <sam(at)samason(dot)me(dot)uk> writes:

>> But there's a kicker: in Subclause 6.12, <cast specification>, in the
>> General Rules is:
>>
>> a) If the <cast operand> specifies NULL, then the result of CS is
>> the null value and no further General Rules of this Subclause
>> are applied.
>>
>> That "no further General Rules" clause implies (assuming it's not a
>> blatant mistake in the spec) that this rule is therefore skipped in
>> the case of nulls:

Sam> I think the NOT NULL constraint is a PG specific constraint, I
Sam> can't see how it's allowed in the spec.

That's a good point; it doesn't seem to be.

But the spec _does_ appear to allow CHECK(VALUE IS NOT NULL) as a
domain constraint (in general the spec defines NOT NULL constraints
this way), and the wording from 6.12 implies that that check is still
skipped in the case of NULLs (so that constraint would stop you
inserting a null into a table column (I think), but not from casting a
null value to the domain type).

Sam> The NOT NULL constraint feels wrong as well, what are the
Sam> semantics of:

Sam> CREATE DOMAIN d AS INTEGER NOT NULL;
Sam> SELECT a.n AS aa, b.n AS bb
Sam> FROM (VALUES (CAST(1 AS d)),(2)) a(n)
Sam> LEFT JOIN (VALUES (CAST(1 AS d))) b(n) ON a.n = b.n;

Sam> in the presence of it? I'm expecting aa and bb both to come out
Sam> as domain "d", but this shouldn't work with what you're saying
Sam> the current semantics should be.

I think that's just another example of Tom's initial comment about how
broken domain "not null" constraints are currently.

--
Andrew (irc:RhodiumToad)

Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> "Sam" == Sam Mason <sam(at)samason(dot)me(dot)uk> writes:
> Sam> The NOT NULL constraint feels wrong as well, what are the
> Sam> semantics of:

> Sam> CREATE DOMAIN d AS INTEGER NOT NULL;
> Sam> SELECT a.n AS aa, b.n AS bb
> Sam> FROM (VALUES (CAST(1 AS d)),(2)) a(n)
> Sam> LEFT JOIN (VALUES (CAST(1 AS d))) b(n) ON a.n = b.n;

> Sam> in the presence of it? I'm expecting aa and bb both to come out
> Sam> as domain "d", but this shouldn't work with what you're saying
> Sam> the current semantics should be.

> I think that's just another example of Tom's initial comment about how
> broken domain "not null" constraints are currently.

Well, the LEFT JOIN case is exactly why I feel that domain not-null
constraints are inherently broken. The only clean way around it is to
decree that the output of a left join is not of the domain type after
all, but of its base type. Which seems to me to be one side effect of
the wording in 4.17.4, though they are extending it to *all* evaluation
contexts not only outer joins.

I haven't yet read the additional material you guys found ...

regards, tom lane

On Tue, Sep 15, 2009 at 02:54:18PM +0100, Andrew Gierth wrote:
> the spec _does_ appear to allow CHECK(VALUE IS NOT NULL) as a
> domain constraint (in general the spec defines NOT NULL constraints
> this way),

Huh, that's a trivial rewrite isn't it. Not sure why it didn't occur to
me that it's just syntax sugar.

> and the wording from 6.12 implies that that check is still
> skipped in the case of NULLs (so that constraint would stop you
> inserting a null into a table column (I think), but not from casting a
> null value to the domain type).

Explicitly ignoring NULL values in CAST expressions seems like a good
feature as well. Although it gives me the feeling that domains are more
and more like a mis-designed feature.

> >>>>> "Sam" == Sam Mason <sam(at)samason(dot)me(dot)uk> writes:
> Sam> The NOT NULL constraint feels wrong as well,

> I think that's just another example of Tom's initial comment about how
> broken domain "not null" constraints are currently.

Hum, given that it's just sugar for more general constraints I'm not
sure if it's the not null constraints that are broken or just the
current interpretation of them. They would do the "right thing" if they
were only checked in a limited number of places that the user was aware
of, which the spec seems to imply is when the user explicitly asks for a
CAST to be performed or when writing into the table.

--
Sam http://samason.me.uk/

Since our shop seems to use domains more than most, I figured I
should comment on this thread.

>Sam Mason <sam(at)samason(dot)me(dot)uk> wrote:
>> On Tue, Sep 15, 2009 at 02:54:18PM +0100, Andrew Gierth wrote:

>> and the wording from 6.12 implies that that check is still
>> skipped in the case of NULLs (so that constraint would stop you
>> inserting a null into a table column (I think), but not from
>> casting a null value to the domain type).
>
> Explicitly ignoring NULL values in CAST expressions seems like a
> good feature as well.

OK by me.

> Although it gives me the feeling that domains are more
> and more like a mis-designed feature.

They have their place, for when you don't really need a new type,
but you want to show that multiple columns contain data from the
same set. My rule of thumb is this -- if it would make sense for
two columns to be compared for equality, there's a very good chance
they belong in the same domain; if not, they probably don't. Using
them helps to document complex databases and helps with portability,
quite aside from the issue of constraints.

> Hum, given that it's just sugar for more general constraints I'm
> not sure if it's the not null constraints that are broken or just
> the current interpretation of them. They would do the "right
> thing" if they were only checked in a limited number of places
> that the user was aware of, which the spec seems to imply is when
> the user explicitly asks for a CAST to be performed or when
> writing into the table.

If that's what the spec says, then +1 from me. The change won't
cause problems here.

-Kevin

Sam Mason wrote:
> On Mon, Sep 14, 2009 at 10:54:07AM -0500, Kevin Grittner wrote:
> > Sam Mason <sam(at)samason(dot)me(dot)uk> wrote:
> > > the deeper problem seems to be that the table was created as:
> > >
> > > create table test (a tstdom);
> > >
> > > and not as:
> > >
> > > create table test (a tstdom not null);
> >
> > Given that tstdom is declared as NOT NULL, is this difference
> > considered a *feature* or is it an implementation quirk?
>
> That's why I pointed it out!
>
> Based on my reading of the SQL spec (and reading about Codd's
> descriptions of domains) I'd say it was a bug/implementation quirk.

Do we need a TODO for this item? Also, I see this odd behavior even
without domains:

test=> CREATE TYPE xx AS (x INT);
CREATE TYPE
test=> CREATE TABLE test4(col1 xx NOT NULL);
CREATE TABLE
test=> INSERT INTO test4 VALUES (ROW(NULL));
INSERT 0 1
test=> SELECT * FROM test4 WHERE col1 IS NULL;
col1
------
()
(1 row)

Here I am illustrating that NOT NULL and IS NULL have different ideas of
what a NULL is? That seems odd too.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Mon, Sep 21, 2009 at 12:31 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Sam Mason wrote:
>> On Mon, Sep 14, 2009 at 10:54:07AM -0500, Kevin Grittner wrote:
>> > Sam Mason <sam(at)samason(dot)me(dot)uk> wrote:
>> > > the deeper problem seems to be that the table was created as:
>> > >
>> > >   create table test (a tstdom);
>> > >
>> > > and not as:
>> > >
>> > >   create table test (a tstdom not null);
>> >
>> > Given that tstdom is declared as NOT NULL, is this difference
>> > considered a *feature* or is it an implementation quirk?
>>
>> That's why I pointed it out!
>>
>> Based on my reading of the SQL spec (and reading about Codd's
>> descriptions of domains) I'd say it was a bug/implementation quirk.
>
> Do we need a TODO for this item?  Also, I see this odd behavior even
> without domains:
>
>        test=> CREATE TYPE xx AS (x INT);
>        CREATE TYPE
>        test=> CREATE TABLE test4(col1 xx NOT NULL);
>        CREATE TABLE
>        test=> INSERT INTO test4 VALUES (ROW(NULL));
>        INSERT 0 1
>        test=> SELECT * FROM test4 WHERE col1 IS NULL;
>         col1
>        ------
>         ()
>        (1 row)
>
> Here I am illustrating that NOT NULL and IS NULL have different ideas of
> what a NULL is?  That seems odd too.

I think what you're demonstrating is that there is a difference
between a NULL, and a row consisting of a single NULL. We've had some
dispute (on this thread) about whether that ought to be the case, but
this is certainly a lot less weird and more debatable than the domain
example, at least IMO.

...Robert