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.

Reply via email to