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 |
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 |