BUG #3822: Nonstandard precedence for comparison operators

Started by Pedro Gimenoover 18 years ago11 messagesbugs
Jump to latest
#1Pedro Gimeno
pgsql-001@personal.formauri.es

The following bug has been logged online:

Bug reference: 3822
Logged by: Pedro Gimeno
Email address: pgsql-001@personal.formauri.es
PostgreSQL version: 8.2.5
Operating system: Any
Description: Nonstandard precedence for comparison operators
Details:

The operators <>, <= and >= are expected to have the same precedence as =, <
and >, but according to the docs (and matching actual behaviour) they have
the same precedence as e.g. ||.

This leads to surprises such as this one:

SELECT 'a' = 'b' || 'c';
SELECT 'a' < 'b' || 'c';
SELECT 'a' > 'b' || 'c';

return either FALSE or TRUE, but

SELECT 'a' <> 'b' || 'c';
SELECT 'a' <= 'b' || 'c';
SELECT 'a' >= 'b' || 'c';

give an error because they are equivalent to:

SELECT ('a' <> 'b') || 'c';
SELECT ('a' <= 'b') || 'c';
SELECT ('a' >= 'b') || 'c';

respectively, which try to concatenate a boolean value with 'c'. On the
other hand, the following work as expected:

SELECT 'b' || 'c' <> 'a';
SELECT 'b' || 'c' >= 'a';
SELECT 'b' || 'c' <= 'a';

That's because, having || and e.g. <> the same priority, they are evaluated
left-to-right.

Of course the same applies to != since it is converted to <>.

Now being picky, the precedence of < and > should be the same as that of =
(for comparison, not for assignment), which makes a difference in rare cases
like this:

SELECT false = false < false;

which in PostgreSQL returns true instead of false because it is equivalent
to this:

SELECT false = (false < false);

-- Pedro Gimeno

#2Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Pedro Gimeno (#1)
Re: BUG #3822: Nonstandard precedence for comparison operators

On Mon, Dec 17, 2007 at 1:58 PM, in message

<200712171958.lBHJwOBb037317@wwwmaster.postgresql.org>, "Pedro Gimeno"
<pgsql-001@personal.formauri.es> wrote:

Description: Nonstandard precedence for comparison operators

The operators <>, <= and >= are expected to have the same precedence as =, <
and >, but according to the docs (and matching actual behaviour) they have
the same precedence as e.g. ||.

This leads to surprises

Of course the same applies to != since it is converted to <>.

Now being picky, the precedence of < and > should be the same as that of =
(for comparison, not for assignment), which makes a difference in rare cases

Everything Pedro said in this post matched my understanding of the
SQL specification. I went back and read those sections closely,
and I can't find any basis for interpreting the spec any other way.
This seems like a pretty serious point of non-compliance to me.

That said, bringing PostgreSQL into compliance with the standard
would undoubtedly break some people's existing applications. It
seems to call for the same phased approach as the standard
conforming string literals, with GUCs to control warnings for
problem constructs and legacy versus standard runtime behavior.

-Kevin

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kevin Grittner (#2)
Re: BUG #3822: Nonstandard precedence for comparison operators

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

That said, bringing PostgreSQL into compliance with the standard
would undoubtedly break some people's existing applications.

The spec seems to barely have a notion of operator precedence at all ---
for example, all the variants of <predicate> are at the same precedence
level, and if I'm reading it right they actively disallow ambiguous
cases by requiring parentheses; note the way that <boolean primary>
is defined. This entire arrangement breaks down as soon as you consider
user-defined operators that yield boolean results. So I'm not
particularly excited about the idea of slavish compliance with the spec
in this area.

To the extent that you do believe the spec, there are more problems with
our precedence rules than just where <= fits --- it looks to me like IS
[NOT] NULL is at the wrong precedence, too. And then there's the whole
question of associativity.

It seems to call for the same phased approach as the standard
conforming string literals, with GUCs to control warnings for problem
constructs and legacy versus standard runtime behavior.

Good luck implementing that --- the precedence is hard-wired into the
bison grammar rules. There are also extremely good reasons for not
having GUC variables that affect parsing behavior.

Given that it's been this way for ten years and no one has complained
before, I'm disinclined to change it, and even more disinclined to
invest the effort that would be involved in letting the behavior vary
at runtime.

regards, tom lane

#4Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Tom Lane (#3)
Re: BUG #3822: Nonstandard precedence for comparison operators

On Thu, Dec 27, 2007 at 4:22 PM, in message <21242.1198794130@sss.pgh.pa.us>,

Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

To the extent that you do believe the spec, there are more problems with
our precedence rules than just where <= fits --- it looks to me like IS
[NOT] NULL is at the wrong precedence, too. And then there's the whole
question of associativity.

Well, surely, the fact that there is more than one problem isn't,
by itself, an argument not to fix any of them.

It seems to call for the same phased approach as the standard
conforming string literals, with GUCs to control warnings for problem
constructs and legacy versus standard runtime behavior.

Good luck implementing that --- the precedence is hard-wired into the
bison grammar rules. There are also extremely good reasons for not
having GUC variables that affect parsing behavior.

But we have done so before, in the interests of converging on the
standard.

Given that it's been this way for ten years and no one has complained
before, I'm disinclined to change it, and even more disinclined to
invest the effort that would be involved in letting the behavior vary
at runtime.

That's a fair point. It's very unlikely to bite an existing
PostgreSQL user very hard. The pain could come in migrating a large
set of complex queries from another DBMS. I don't recall seeing any
migration document to help people identify these issues up front,
but we should probably have one (if we don't) and this should
probably be mentioned.

Based on this discussion, I'll remind our staff to be generous in
the use of parentheses when doing ad hoc queries.

-Kevin

#5Pedro Gimeno
pgsql-001@personal.formauri.es
In reply to: Tom Lane (#3)
Re: BUG #3822: Nonstandard precedence for comparison operators

Tom Lane wrote:

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

That said, bringing PostgreSQL into compliance with the standard
would undoubtedly break some people's existing applications.

I wonder how that compares to broken queries while migrating databases
from other systems. I'd bet there are more of the latter. We ran into
that while running a query like: SELECT ... WHERE column <>
another_column || 'literal';, variants of which I think can be
relatively common compared to e.g. applications that build a boolean
array using expr1 <> expr2 || boolean_value.

The spec seems to barely have a notion of operator precedence at all ---

The precedence is given by the parse tree and is well defined. Perhaps
it may vary for a given operator depending on the context but it's
clearly different to the one PostgreSQL is using in the examples I gave.

for example, all the variants of <predicate> are at the same precedence
level, and if I'm reading it right they actively disallow ambiguous
cases by requiring parentheses; note the way that <boolean primary>
is defined. This entire arrangement breaks down as soon as you consider
user-defined operators that yield boolean results. So I'm not
particularly excited about the idea of slavish compliance with the spec
in this area.

Note that we considered PostgreSQL precisely because of its high
standards compliance and this problem has been a bit disappointing, even
more given that I looked for information on what nonstandard bits did
PostgreSQL have and didn't see this.

In the case of user defined operators I'd expect them to have a fixed
precedence regardless of the semantics and that can be different
depending on the operator.

Given that it's been this way for ten years and no one has complained
before, I'm disinclined to change it, and even more disinclined to
invest the effort that would be involved in letting the behavior vary
at runtime.

It has often happened that a new version has caused the need of porting
code, that's not new. Users will very likely appreciate compliance with
the standard even with the hassle of porting the applications, specially
when the fixes, if they're necessary, can easily be made backwards
compatible by using parentheses. For that reason I don't think a runtime
selection of behaviour would be neecessary in this case.

-- Pedro Gimeno

#6Michael Glaesemann
grzm@seespotcode.net
In reply to: Pedro Gimeno (#5)
Re: BUG #3822: Nonstandard precedence for comparison operators

On Dec 29, 2007, at 14:09 , Pedro Gimeno wrote:

variants of which I think can be
relatively common compared to e.g. applications that build a boolean
array using expr1 <> expr2 || boolean_value.

I'm probably being dense, but I don't see how this is an issue. || is
string concatenation, not a logical OR. You're going to throw an
error because || isn't a boolean operator, not because of any strange
precedence rules.

test=# select 1 <> 2 || true;
ERROR: operator does not exist: boolean || boolean
LINE 1: select 1 <> 2 || true;
^
HINT: No operator matches the given name and argument type(s). You
may need to add explicit type casts.

test=# select 'foo'::text <> 'bar'::text || true;
ERROR: operator does not exist: boolean || boolean
LINE 1: select 'foo'::text <> 'bar'::text || true;
^
HINT: No operator matches the given name and argument type(s). You
may need to add explicit type casts.

Michael Glaesemann
grzm seespotcode net

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Glaesemann (#6)
Re: BUG #3822: Nonstandard precedence for comparison operators

Michael Glaesemann <grzm@seespotcode.net> writes:

I'm probably being dense, but I don't see how this is an issue.

That's just a bug in his example ;-)

The real question is whether there is enough of a problem here to
justify creating new problems, in the form of backwards-compatibility
hazards. One complaint in ten years tells me there's not. We know
what the SQL-portability hot-button issues are, because we have gotten
repeated complaints about them --- identifier case-folding and
backslashes in string literals are two that spring to mind instantly.
If operator precedence were an issue worth spending time on, it would
have come up before, repeatedly.

I would not actually object to making small tweaks in the precedence
rules to move a little closer to spec compliance; it undoubtedly is
just plain weird that = and <> have their own precedence rules.
However, "closer to spec" is a lot weaker argument than "matches spec",
and I really don't think that we want to try to match the spec exactly
in this area.

What I do *not* want to do is invest the level of effort suggested
by Kevin, with two grammars or whatever it would take to make that
happen. This problem is not worth that.

regards, tom lane

#8Pedro Gimeno
pgsql-001@personal.formauri.es
In reply to: Tom Lane (#7)
Re: BUG #3822: Nonstandard precedence for comparison operators

Tom Lane wrote:

Michael Glaesemann <grzm@seespotcode.net> writes:

I'm probably being dense, but I don't see how this is an issue.

That's just a bug in his example ;-)

Right, sorry. I meant boolean_array rather than boolean_value:

=> SELECT 'a' <> 'a' || '{true}';
?column?
----------
{f,t}
(1 row)

-- Pedro Gimeno

#9Bruce Momjian
bruce@momjian.us
In reply to: Pedro Gimeno (#5)
Re: BUG #3822: Nonstandard precedence for comparison operators

This has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Pedro Gimeno wrote:

Tom Lane wrote:

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

That said, bringing PostgreSQL into compliance with the standard
would undoubtedly break some people's existing applications.

I wonder how that compares to broken queries while migrating databases
from other systems. I'd bet there are more of the latter. We ran into
that while running a query like: SELECT ... WHERE column <>
another_column || 'literal';, variants of which I think can be
relatively common compared to e.g. applications that build a boolean
array using expr1 <> expr2 || boolean_value.

The spec seems to barely have a notion of operator precedence at all ---

The precedence is given by the parse tree and is well defined. Perhaps
it may vary for a given operator depending on the context but it's
clearly different to the one PostgreSQL is using in the examples I gave.

for example, all the variants of <predicate> are at the same precedence
level, and if I'm reading it right they actively disallow ambiguous
cases by requiring parentheses; note the way that <boolean primary>
is defined. This entire arrangement breaks down as soon as you consider
user-defined operators that yield boolean results. So I'm not
particularly excited about the idea of slavish compliance with the spec
in this area.

Note that we considered PostgreSQL precisely because of its high
standards compliance and this problem has been a bit disappointing, even
more given that I looked for information on what nonstandard bits did
PostgreSQL have and didn't see this.

In the case of user defined operators I'd expect them to have a fixed
precedence regardless of the semantics and that can be different
depending on the operator.

Given that it's been this way for ten years and no one has complained
before, I'm disinclined to change it, and even more disinclined to
invest the effort that would be involved in letting the behavior vary
at runtime.

It has often happened that a new version has caused the need of porting
code, that's not new. Users will very likely appreciate compliance with
the standard even with the hassle of porting the applications, specially
when the fixes, if they're necessary, can easily be made backwards
compatible by using parentheses. For that reason I don't think a runtime
selection of behaviour would be neecessary in this case.

-- Pedro Gimeno

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Pedro Gimeno (#5)
Re: BUG #3822: Nonstandard precedence for comparison operators

Pedro Gimeno wrote:

The spec seems to barely have a notion of operator precedence at all ---

The precedence is given by the parse tree and is well defined. Perhaps
it may vary for a given operator depending on the context but it's
clearly different to the one PostgreSQL is using in the examples I gave.

It would be interesting if someone could work out an operator precedence table
that the spec would require, so we can contrast that with the one we
currently implement. At the moment I couldn't really tell how extensive the
problem is. Even if we change nothing, this would be useful for
documentation.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

#11Bruce Momjian
bruce@momjian.us
In reply to: Pedro Gimeno (#5)
Re: BUG #3822: Nonstandard precedence for comparison operators

Added to TODO:

* Fix inconsistent precedence of =, >, and < compared to <>, >=, and <=

http://archives.postgresql.org/pgsql-bugs/2007-12/msg00145.php

---------------------------------------------------------------------------

Pedro Gimeno wrote:

Tom Lane wrote:

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

That said, bringing PostgreSQL into compliance with the standard
would undoubtedly break some people's existing applications.

I wonder how that compares to broken queries while migrating databases
from other systems. I'd bet there are more of the latter. We ran into
that while running a query like: SELECT ... WHERE column <>
another_column || 'literal';, variants of which I think can be
relatively common compared to e.g. applications that build a boolean
array using expr1 <> expr2 || boolean_value.

The spec seems to barely have a notion of operator precedence at all ---

The precedence is given by the parse tree and is well defined. Perhaps
it may vary for a given operator depending on the context but it's
clearly different to the one PostgreSQL is using in the examples I gave.

for example, all the variants of <predicate> are at the same precedence
level, and if I'm reading it right they actively disallow ambiguous
cases by requiring parentheses; note the way that <boolean primary>
is defined. This entire arrangement breaks down as soon as you consider
user-defined operators that yield boolean results. So I'm not
particularly excited about the idea of slavish compliance with the spec
in this area.

Note that we considered PostgreSQL precisely because of its high
standards compliance and this problem has been a bit disappointing, even
more given that I looked for information on what nonstandard bits did
PostgreSQL have and didn't see this.

In the case of user defined operators I'd expect them to have a fixed
precedence regardless of the semantics and that can be different
depending on the operator.

Given that it's been this way for ten years and no one has complained
before, I'm disinclined to change it, and even more disinclined to
invest the effort that would be involved in letting the behavior vary
at runtime.

It has often happened that a new version has caused the need of porting
code, that's not new. Users will very likely appreciate compliance with
the standard even with the hassle of porting the applications, specially
when the fixes, if they're necessary, can easily be made backwards
compatible by using parentheses. For that reason I don't think a runtime
selection of behaviour would be neecessary in this case.

-- Pedro Gimeno

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +