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"?
>> transform_null_equals_options[] = { [...]
>>
>> 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.
>> Or anything in better English.
>
> I've changed this, and hope it's clearer.
Yep, and more elegant than my proposal.
>> * 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?
>> 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.
--
Fabien.
