Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-26 Thread Tom Lane
Peter Eisentraut  writes:
> On 7/26/16 7:46 PM, Thomas Munro wrote:
>> By the way, our documentation says that NOT NULL constraints are
>> equivalent to CHECK (column_name IS NOT NULL).  That is what the SQL
>> standard says, but in fact our NOT NULL constraints are equivalent to
>> CHECK (column_name IS DISTINCT FROM NULL).  Should we update the
>> documentation with something like the attached?

> Couldn't we just fix that instead?  For NOT NULL constraints on
> composite type columns, create a full CHECK (column_name IS NOT NULL)
> constraint instead, foregoing the attnotnull optimization.

Maybe.  There's a patch floating around that expands attnotnull into
CHECK constraints, which'd provide the infrastructure needed to consider
changing this behavior.  But that's not going to be back-patchable, and
as I noted in <10682.1469566...@sss.pgh.pa.us>, we have a problem right
now that the planner's constraint exclusion logic gets these semantics
wrong.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-26 Thread Peter Eisentraut
On 7/26/16 7:46 PM, Thomas Munro wrote:
> By the way, our documentation says that NOT NULL constraints are
> equivalent to CHECK (column_name IS NOT NULL).  That is what the SQL
> standard says, but in fact our NOT NULL constraints are equivalent to
> CHECK (column_name IS DISTINCT FROM NULL).  Should we update the
> documentation with something like the attached?

Couldn't we just fix that instead?  For NOT NULL constraints on
composite type columns, create a full CHECK (column_name IS NOT NULL)
constraint instead, foregoing the attnotnull optimization.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-26 Thread Thomas Munro
On Wed, Jul 27, 2016 at 7:35 AM, Tom Lane  wrote:
> "David G. Johnston"  writes:
>> The concept embodied by "NULL" in the operator "IS [NOT] NULL" is distinct
>> from the concept embodied by "NULL" in the operator "IS [NOT] DISTINCT
>> FROM".
>
>> In short, the former smooths out the differences between composite and
>> non-composite types while the later maintains their differences.  While a
>> bit confusing I don't see that there is much to be done about it - aside
>> from making the distinction more clear at:
>> https://www.postgresql.org/docs/devel/static/functions-comparison.html
>
>> Does spec support or refute this distinction in treatment?
>
> AFAICS, the IS [NOT] DISTINCT FROM operator indeed is specified to do the
> "obvious" thing when one operand is NULL: you get a simple nullness check
> on the other operand.  So I went ahead and documented that it could be
> used for that purpose.

By the way, our documentation says that NOT NULL constraints are
equivalent to CHECK (column_name IS NOT NULL).  That is what the SQL
standard says, but in fact our NOT NULL constraints are equivalent to
CHECK (column_name IS DISTINCT FROM NULL).  Should we update the
documentation with something like the attached?

-- 
Thomas Munro
http://www.enterprisedb.com


not-null-does-not-mean-check-is-not-null.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-26 Thread Tom Lane
"David G. Johnston"  writes:
> The concept embodied by "NULL" in the operator "IS [NOT] NULL" is distinct
> from the concept embodied by "NULL" in the operator "IS [NOT] DISTINCT
> FROM".

> In short, the former smooths out the differences between composite and
> non-composite types while the later maintains their differences.  While a
> bit confusing I don't see that there is much to be done about it - aside
> from making the distinction more clear at:
> ​https://www.postgresql.org/docs/devel/static/functions-comparison.html

> Does spec support or refute this distinction in treatment?

AFAICS, the IS [NOT] DISTINCT FROM operator indeed is specified to do the
"obvious" thing when one operand is NULL: you get a simple nullness check
on the other operand.  So I went ahead and documented that it could be
used for that purpose.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-26 Thread David G. Johnston
On Tue, Jul 26, 2016 at 11:10 AM, Tom Lane  wrote:

>
> 3. Andrew also revived the bug #7808 thread in which it was complained
> that ExecMakeTableFunctionResult should not fail on null results from
> functions returning SETOF composite.  That's related only in that the
> proposed fix is to translate a plain NULL into ROW(NULL,NULL,...).
> I do not much like the implementation details of his proposed patch,
> but I think making that translation is probably better than failing
> altogether.  Given the infrequency of field complaints, my recommendation
> here is to fix it in HEAD but not back-patch.
>

​Andrew's response also introduces an area for documentation improvement.

The concept embodied by "NULL" in the operator "IS [NOT] NULL" is distinct
from the concept embodied by "NULL" in the operator "IS [NOT] DISTINCT
FROM".

In short, the former smooths out the differences between composite and
non-composite types while the later maintains their differences.  While a
bit confusing I don't see that there is much to be done about it - aside
from making the distinction more clear at:

​https://www.postgresql.org/docs/devel/static/functions-comparison.html

Does spec support or refute this distinction in treatment?

CREATE TYPE twocol AS (col1 text, col2 int);
CREATE TYPE twocolalt AS (col1 text, col2 int);

SELECT
row(null, null)::twocol IS NULL,
null::twocol IS NULL,
null::twocol IS NOT DISTINCT FROM row(null, null)::twocol

Its at least worth considering whether to note that when comparing two
composite values using IS DISTINCT FROM that the comparison is unaware of
the composite type itself but performs an iterative comparison of each pair
of columns.

SELECT row(null, null)::twocol IS NOT DISTINCT FROM row(null,
null)::twocolalt

This is likely to matter little in practice given low odds of someone
accidentially comparing two physically identical but semantically different
composite types.

David J.




​


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-26 Thread Tom Lane
Peter Eisentraut  writes:
> On 7/22/16 7:01 PM, Andrew Gierth wrote:
>> In light of the fact that it is an endless cause of bugs both in pg and
>> potentially to applications, I propose that we cease attempting to
>> conform to the spec's definition of IS NULL in favour of the following
>> rules:

> I can't see how we would incompatibly change existing
> standards-conforming behavior merely because users are occasionally
> confused and there are some bugs in corner cases.

It seems clear that Andrew's proposal to reject the spec's definition that
ROW(NULL,NULL,...) IS NULL is true has failed.  So ExecEvalNullTest()
should keep on doing what it's doing.  There are several related issues
however:

1. As per bug #14235, eval_const_expressions() behaves differently from
ExecEvalNullTest() for nested-composite-types cases.  It is inarguably
a bug that they don't give the same answers.  So far, no one has spoken
against the conclusion reached in that thread that ExecEvalNullTest()
correctly implements the spec's semantics and eval_const_expressions()
does not.  Therefore I propose to apply and back-patch Andrew's fix from
that thread.

2. Our (or at least my) previous understanding was that ROW(NULL,NULL,...)
ought to be considered NULL for all purposes, and that our failure to
do so anywhere except ExecEvalNullTest was a spec violation we should do
something about someday.  But the bug #14235 thread makes it clear that
that's not so, and that only in very limited cases is there an argument
for applying IS [NOT] NULL's behavior to other constructs.  COALESCE()
was mentioned as something that maybe should change.  I'm inclined to vote
"no, let's keep COALESCE as-is".  The spec's use of, essentially, macro
expansion to define COALESCE is just stupid, not least because it's
impossible to specify the expected at-most-once evaluation of each
argument expression that way.  (They appear to try to dodge that question
by forbidding argument expressions with side-effects, which is just lame.)
But had they written out a definition of COALESCE's behavior in words,
they would almost certainly have written "V is not the null value" not
"V IS NOT NULL".  Anyone who stopped to think about it would surely think
that the result of COALESCE(ROW(1,NULL), NULL) should not be NULL.  So I
think the apparent mandate to use IS NOT NULL's semantics for rowtype
values in COALESCE is just an artifact of careless drafting.  Between that
and the backwards-compatibility hazards of changing, it's not worth it.

3. Andrew also revived the bug #7808 thread in which it was complained
that ExecMakeTableFunctionResult should not fail on null results from
functions returning SETOF composite.  That's related only in that the
proposed fix is to translate a plain NULL into ROW(NULL,NULL,...).
I do not much like the implementation details of his proposed patch,
but I think making that translation is probably better than failing
altogether.  Given the infrequency of field complaints, my recommendation
here is to fix it in HEAD but not back-patch.

Comments?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-26 Thread Kevin Grittner
On Mon, Jul 25, 2016 at 8:28 PM, Peter Eisentraut
 wrote:
> On 7/22/16 7:01 PM, Andrew Gierth wrote:
>> In light of the fact that it is an endless cause of bugs both in pg and
>> potentially to applications, I propose that we cease attempting to
>> conform to the spec's definition of IS NULL in favour of the following
>> rules:
>
> I can't see how we would incompatibly change existing
> standards-conforming behavior merely because users are occasionally
> confused and there are some bugs in corner cases.

+1

Anywhere we have standard-conforming behavior, changing the
semantics of the standard syntax seems insane.  We can create new
syntax for extensions where we are convinced we have a better idea.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-25 Thread Peter Eisentraut
On 7/22/16 7:01 PM, Andrew Gierth wrote:
> In light of the fact that it is an endless cause of bugs both in pg and
> potentially to applications, I propose that we cease attempting to
> conform to the spec's definition of IS NULL in favour of the following
> rules:

I can't see how we would incompatibly change existing
standards-conforming behavior merely because users are occasionally
confused and there are some bugs in corner cases.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread Andrew Gierth
> "Andrew" == Andrew Gierth  writes:

 >>> Whole-row vars when constructed never contain the null value.

 David> ...but what does this mean in end-user terms?​

 Andrew> It means for example that this query:

 Andrew>   select y from x left join y on (x.id=y.id) where y is null;

 Andrew> would always return 0 rows.

On second thoughts I'll take this one back. Specifying that whole-row
vars don't contain the null value when constructed doesn't actually
result in no rows, since the construction is logically below the join;
and hence even with that rule in place, the query would return a row for
each non-matched "x" row.

-- 
Andrew (irc:RhodiumToad)


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread Jim Nasby

On 7/22/16 8:05 PM, David G. Johnston wrote:


I would expect that >95% of cases where someone has written (x IS NOT
NULL) for x being a composite type, it's actually a bug and that NOT (x
IS NULL) was intended.


Yeah, it would need to be targeted there.  I agree with the numbers and
the sentiment but it's still allowed and defined behavior which changing
invisibly is disconcerting.


Yeah, that worries me too... I'm not sure what can be done about it 
other than a compatibility GUC (and we know how we love those... :( ).


On the LEFT JOIN scenario, I'm not sure why we should disallow that. Is 
that commonly error prone?


BTW, one thing that would be very nice about this is it'd let you do
DECLARE
  r_new record := coalesce(NEW,OLD);
  r_old record := coalesce(OLD,NEW);

which would be much nicer than how things work today (I'm going on the 
assumption that referencing NEW in a DELETE trigger would be legit and 
give you NULL with the new semantics, but trying to actually reference 
any of it's fields would produce an error).

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread David G. Johnston
On Friday, July 22, 2016, Andrew Gierth  wrote:

> > "David" == David G Johnston  > writes:
>
>  >> Prohibiting IS NOT NULL is not on the cards; it's very widely used.
>
>  David> ​Yet changing how it behaves, invisibly, is?
>
> Did you mean prohibiting it only for composite-type args? It's obviously
> widely used for non-composite args.
>
> I would expect that >95% of cases where someone has written (x IS NOT
> NULL) for x being a composite type, it's actually a bug and that NOT (x
> IS NULL) was intended.
>
>
Yeah, it would need to be targeted there.  I agree with the numbers and the
sentiment but it's still allowed and defined behavior which changing
invisibly is disconcerting.

David J.


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread Andrew Gierth
> "David" == David G Johnston  writes:

 >> Prohibiting IS NOT NULL is not on the cards; it's very widely used.

 David> ​Yet changing how it behaves, invisibly, is?

Did you mean prohibiting it only for composite-type args? It's obviously
widely used for non-composite args.

I would expect that >95% of cases where someone has written (x IS NOT
NULL) for x being a composite type, it's actually a bug and that NOT (x
IS NULL) was intended.

-- 
Andrew (irc:RhodiumToad)


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread David G. Johnston
On Fri, Jul 22, 2016 at 8:04 PM, Andrew Gierth 
wrote:

> > "David" == David G Johnston  writes:
>
>  >> 2. x IS NOT NULL  if and only if  NOT (x IS NULL)
>
>  David> ​I would rather prohibit "IS NOT NULL" altogether.​ If one needs
>  David> to test "NOT (x IS NULL)" they can write it that way.
>
> Prohibiting IS NOT NULL is not on the cards; it's very widely used.
>
>
​Yet changing how it behaves, invisibly, is?  I'm tending to agree that
status-quo is preferable to either option but if you say change is
acceptable I'd say we should do it visibly.

Allowing syntax that is widely used but implementing it differently than
how it is being used seems worse than telling people said syntax is
problematic and we've chosen to avoid the issue altogether.


>  >> Whole-row vars when constructed never contain the null value.
>
>  David> ...but what does this mean in end-user terms?​
>
> It means for example that this query:
>
>   select y from x left join y on (x.id=y.id) where y is null;
>
> would always return 0 rows.
>
>
​Ok, so I'm pretty sure I disagree on this one too.

David J.


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread Andrew Gierth
> "David" == David G Johnston  writes:

 >> 1. x IS NULL  is true if and only if x has the null value (isnull set).

 David> ​I don't have a problem conforming to "ROW(NULL, NULL) IS NULL"
 David> being true...​if you somehow get a hold of something in that
 David> form, which your others points address.

This seems harmless, but I think it's not worth the pain.

I'm informed that for example on Oracle:

  select case when foo_type(null, null) is null
  then 'true' else 'false'
  end from dual;

returns 'false'.

-- 
Andrew (irc:RhodiumToad)


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread Andrew Gierth
> "David" == David G Johnston  writes:

 >> 2. x IS NOT NULL  if and only if  NOT (x IS NULL)

 David> ​I would rather prohibit "IS NOT NULL" altogether.​ If one needs
 David> to test "NOT (x IS NULL)" they can write it that way.

Prohibiting IS NOT NULL is not on the cards; it's very widely used.

 >> Whole-row vars when constructed never contain the null value.

 David> ...but what does this mean in end-user terms?​

It means for example that this query:

  select y from x left join y on (x.id=y.id) where y is null;

would always return 0 rows.

-- 
Andrew (irc:RhodiumToad)


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread David G. Johnston
On Fri, Jul 22, 2016 at 7:01 PM, Andrew Gierth 
wrote:

> In light of the fact that it is an endless cause of bugs both in pg and
> potentially to applications, I propose that we cease attempting to
> conform to the spec's definition of IS NULL in favour of the following
> rules:
>
> 1. x IS NULL  is true if and only if x has the null value (isnull set).
>

​I don't have a problem conforming to "ROW(NULL, NULL) IS NULL" being
true...​if you somehow get a hold of something in that form, which your
others points address.


> 2. x IS NOT NULL  if and only if  NOT (x IS NULL)
>

​I would rather prohibit "IS NOT NULL" altogether.​  If one needs to test
"NOT (x IS NULL)" they can write it that way.

3. ROW() and other row constructors never return the null value.
>

​I think I get this (though if they return row(null, null) I'd say there is
not difference as far as the user is conconcerned)...


> Whole-row vars when constructed never contain the null value.
>

...but what does this mean in end-user terms?​


> 4. Columns or variables of composite type can (if not declared NOT NULL)
>
contain the null value (isnull set) which is distinct from an
> all-columns-null value.
>

Is this just about the validation of the component types; which seems only
to be realized via DOMAINs?  If not I don't follow how this applies or is
different from what we do today.


> 5. COALESCE(x,y) continues to return y if and only if x is the null
> value. (We currently violate the spec here.)
>

​I would concur - especially if in your referenced example
COALESCE((null,1),(2,null)) indeed would have to return (2,null​)

My comment to #1 implies that I think COALESCE((null,null),(2,null)) should
return (2,null)...I am OK with that.  Operationally (null,null) should be
indistinguishable from the null value.  It mostly is today and we should
identify and fix those areas where they are different - not work to make
them more distinguishable.


>
> (X. Optionally, consider adding new predicates:
>
>   x IS ALL NULL
>   x IS NOT ALL NULL
>   x IS ALL NOT NULL
>   x IS NOT ALL NOT NULL
>
> which would examine the fields of x non-recursively.)
>
>
​Not sure regarding recursion here but I'd much rather work a way to fit
this into the existing ANY syntax:

NULL IS ANY(x) -- definitely needs some bike-shedding though...

​This presupposes that ROW(null, null) and null are indistinguishable
operationally which makes the "ALL" form unnecessary; and ANY = NOT(ALL)

David J.


[HACKERS] Proposal: revert behavior of IS NULL on row types

2016-07-22 Thread Andrew Gierth
In light of the fact that it is an endless cause of bugs both in pg and
potentially to applications, I propose that we cease attempting to
conform to the spec's definition of IS NULL in favour of the following
rules:

1. x IS NULL  is true if and only if x has the null value (isnull set).

2. x IS NOT NULL  if and only if  NOT (x IS NULL)

3. ROW() and other row constructors never return the null value.
Whole-row vars when constructed never contain the null value. 

4. Columns or variables of composite type can (if not declared NOT NULL)
contain the null value (isnull set) which is distinct from an
all-columns-null value.

5. COALESCE(x,y) continues to return y if and only if x is the null
value. (We currently violate the spec here.)

(X. Optionally, consider adding new predicates:

  x IS ALL NULL
  x IS NOT ALL NULL
  x IS ALL NOT NULL
  x IS NOT ALL NOT NULL

which would examine the fields of x non-recursively.)

Justification:

https://www.postgresql.org/message-id/4f6a90a0-c6e8-22eb-3b7a-727f8a60f3b1%40BlueTreble.com
https://www.postgresql.org/message-id/20160708024746.1410.57282%40wrigleys.postgresql.org

Further rationale:

https://www.postgresql.org/message-id/87zip9qti4.fsf%40news-spur.riddles.org.uk

-- 
Andrew (irc:RhodiumToad)


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers