Re: Make foo=null a warning by default.

Поиск
Список
Период
Сортировка
От Fabien COELHO
Тема Re: Make foo=null a warning by default.
Дата
Msg-id alpine.DEB.2.21.1807160910570.3767@lancre
обсуждение исходный текст
Ответ на Make foo=null a warning by default.  (David Fetter <david@fetter.org>)
Ответы Re: Make foo=null a warning by default.  (David Fetter <david@fetter.org>)
Список pgsql-hackers
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.


В списке pgsql-hackers по дате отправления:

Предыдущее
От: Heikki Linnakangas
Дата:
Сообщение: Re: Make foo=null a warning by default.
Следующее
От: Fabien COELHO
Дата:
Сообщение: Re: Make foo=null a warning by default.