Re: Make foo=null a warning by default.

Hello David,

> Per a discussion with Andrew Gierth and Vik Fearing, both of whom
> helped make this happen, please find attached a patch which makes it
> possible to get SQL standard behavior for "= NULL", which is an error.
> It's been upgraded to a warning, and can still be downgraded to
> silence (off) and MS-SQL-compatible behavior (on).

That's nice, and good for students, and probably others as well:-)

A few comments:

Patch applies & compiles, "make check" ok.

#backslash_quote = safe_encoding # on, off, or safe_encoding
[...]
#transform_null_equals = warn

I think it would be nice to have the possible values as a comment in
"postgresql.conf".

* Code:

-bool operator_precedence_warning = false;
+bool operator_precedence_warning = false;

Is this reindentation useful/needed?

+ parser_errposition(pstate, a->location)));
+ if (need_transform_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ON)

For consistency, maybe skip a line before the if?

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.

Guc help text: ISTM that it should spell out the possible values and their
behavior. Maybe something like:

"""
When turned on, turns expr = NULL into expr IS NULL.
With off, warn or error, do not do so and be silent, issue a warning or an error.
The standard behavior is not to perform this transformation.
Default is warn.
"""

Or anything in better English.

* 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];

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

Maybe the behavior could be retested after the reset?

* Documentation:

The "error" value is not documented.

More generally, The value is said to be an enum, but the list of values is
not listed anywhere in the documentation.

ISTM that the doc would be clearer if for each of the four values of the
parameter the behavior is spelled out.

Maybe "warn" in the doc should be <literal>warn</literal> or something.

--
Fabien.