Re: Make foo=null a warning by default.

From: David Fetter <david(at)fetter(dot)org>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Make foo=null a warning by default.
Date: 2018-07-18 04:11:23
Message-ID: 20180718041123.GZ22932@fetter.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 17, 2018 at 08:34:17AM -0400, Fabien COELHO wrote:
>
> Hello David,
>
> A few comments about this v2.
>
> ISTM that there is quite strong opposition to having "warn" as a default, so
> probably you should set if "off"?

Done.

> >>I do not really understand the sort order of this array. Maybe it could be
> >>alphabetical, or per value? Or maybe it is standard values and then
> >>synonyms, in which is case maybe a comment on the end of the list.
> >
> >I've changed it to per value. Is this better?
>
> At least, I can see the logic.

I've now ordered it in what seems like a reasonable way, namely all
the "off" options, then the "on" option.

> >>Or anything in better English.
> >
> >I've changed this, and hope it's clearer.
>
> Yep, and more elegant than my proposal.

I assure you that you expression yourself in English a good deal
better than I do in Portuguese.

> >>* Test
> >>
> >> +select 1=null;
> >> + ?column?
> >>
> >>Maybe use as to describe the expected behavior, so that it is easier to
> >>check, and I think that "?column?" looks silly eg:
> >>
> >> select 1=null as expect_{true,false}[_and_a_warning/error];
> >
> >Changed to more descriptive headers.
>
> Cannot see it in the patch update?

Oops. They should be there now.

> >> create table cnullchild (check (f1 = 1 or f1 = null)) inherits(cnullparent);
> >> +WARNING: = NULL can only produce a NULL
> >>
> >>I'm not sure of this test case. Should it be turned into "is null"?
> >
> >This was just adjusting the existing test output to account for the
> >new warning. I presume it was phrased that way for a reason.
>
> Hmmm. Probably you are right. I think that the test case deserves better
> comments, as it is most peculiar. It was added by Tom for testing CHECK
> constant NULL expressions simplications.
>
> TRUE OR NULL is TRUE, but FALSE OR NULL is NULL. Why not. Combined with the
> fact that NULL is considered ok for a CHECK, this lead to strange but
> intentional behavior, such as:
>
> -- this CHECK is always nignored in practice
> CREATE TABLE foo (i INT CHECK(i = 1 OR NULL));
> INSERT INTO foo(i) VALUES(2); # ACCEPTED
>
> -- but this one is not
> CREATE TABLE foo (i INT CHECK(i = 1 OR FALSE));
> INSERT INTO foo(i) VALUES(2); # REFUSED
>
> Can't say I'm thrilled about that, and the added warning looks appropriate.

Per request, the warning is off by default, so the warning went away.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Attachment Content-Type Size
0001-Make-transform_null_equals-into-an-enum.patch text/x-diff 11.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-07-18 04:47:11 More consistency for some file-related error message
Previous Message Kyotaro HORIGUCHI 2018-07-18 04:10:20 Re: [bug fix] Produce a crash dump before main() on Windows