Re: [HACKERS] Precedence of standard comparison operators
On 10 August 2015 at 16:31, Tom Lane wrote: > Pavel Stehule writes: > >> On Sun, Aug 09, 2015 at 08:06:11PM -0400, Tom Lane wrote: > >>> So yeah, I do think that getting a syntax error if you don't use > >>> parentheses is the preferable behavior here. > > > If we raise a syntax error, then there should be very informative > message, > > Yeah, it would sure be nice to throw something better than "syntax error". > But I've looked at bison's facilities for that, and they're pretty bad. > I'm not sure there's much we can do short of moving to some other parser > generator tool, which would be a Large Undertaking ... and I don't know > of any arguably-better tool anyway. > I would say that anyone who's tricksy enough to be using boolean logic in the way you describe would be expected to have enough nouse about them to either a) know what the precedences are or b) know that their lack of knowledge means they should sprinkle their code with brackets . Returning a syntax error for something that isn't actually an error in syntax is a poor showing. Are we to start expecting syntax errors when people use comparison operators on NULLable columns without a COALESCE because the comparison might do something they don't expect if they haven't tested their code with NULL values ? IMO using a = b < c as described is unnecessarily* horrid, *whichever* way you mean it, and will only produce the sort of unreadability that generates errors in the long run anyway (even if you understand it, chances are the next poor sap reading your code won't) and deserves code that breaks, especially if you're the sort of person who uses it without fully understanding it. (*Unnecessarily because there are clearer constructs - CASE springs to mind - that do the same thing without the associated unreadability and/or ambiguity) Geoff
Re: [HACKERS] Precedence of standard comparison operators
Pavel Stehule writes: >> On Sun, Aug 09, 2015 at 08:06:11PM -0400, Tom Lane wrote: >>> So yeah, I do think that getting a syntax error if you don't use >>> parentheses is the preferable behavior here. > If we raise a syntax error, then there should be very informative message, Yeah, it would sure be nice to throw something better than "syntax error". But I've looked at bison's facilities for that, and they're pretty bad. I'm not sure there's much we can do short of moving to some other parser generator tool, which would be a Large Undertaking ... and I don't know of any arguably-better tool anyway. 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] Precedence of standard comparison operators
2015-08-10 5:37 GMT+02:00 Noah Misch : > On Sun, Aug 09, 2015 at 08:06:11PM -0400, Tom Lane wrote: > > Noah Misch writes: > > > In SQL:2008 and SQL:2011 at least, "=", "<" and "BETWEEN" are all in > the same > > > boat. They have no precedence relationships to each other; SQL > sidesteps the > > > question by requiring parentheses. They share a set of precedence > > > relationships to other constructs. SQL does not imply whether to put > them in > > > one %nonassoc precedence group or in a few, but we can contemplate > whether > > > users prefer an error or prefer the 9.4 behavior for affected queries. > > > > Part of my thinking was that the 9.4 behavior fails the principle of > least > > astonishment, because I seriously doubt that people expect '=' to be > > either right-associative or lower priority than '<'. Here's one example: > > > > regression=# select false = true < false; > > ?column? > > -- > > t > > (1 row) > > > So yeah, I do think that getting a syntax error if you don't use > > parentheses is the preferable behavior here. > If we raise a syntax error, then there should be very informative message, because pattern true = 2 > 1 is probably relative often and it is hard to find syntax error on this trivial expression Regards Pavel > > I agree. > > > -- > 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] Precedence of standard comparison operators
On Sun, Aug 09, 2015 at 08:06:11PM -0400, Tom Lane wrote: > Noah Misch writes: > > In SQL:2008 and SQL:2011 at least, "=", "<" and "BETWEEN" are all in the > > same > > boat. They have no precedence relationships to each other; SQL sidesteps > > the > > question by requiring parentheses. They share a set of precedence > > relationships to other constructs. SQL does not imply whether to put them > > in > > one %nonassoc precedence group or in a few, but we can contemplate whether > > users prefer an error or prefer the 9.4 behavior for affected queries. > > Part of my thinking was that the 9.4 behavior fails the principle of least > astonishment, because I seriously doubt that people expect '=' to be > either right-associative or lower priority than '<'. Here's one example: > > regression=# select false = true < false; > ?column? > -- > t > (1 row) > So yeah, I do think that getting a syntax error if you don't use > parentheses is the preferable behavior here. I agree. -- 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] Precedence of standard comparison operators
Noah Misch writes: > On Sun, Aug 09, 2015 at 07:16:02PM -0400, Tom Lane wrote: >> Noah Misch writes: >>> It does risk that. Same deal with making "=" have the same precedence as >>> "<" >>> instead of keeping it slightly lower. >> Agreed, but in that case I think our hand is forced by the SQL standard. > In SQL:2008 and SQL:2011 at least, "=", "<" and "BETWEEN" are all in the same > boat. They have no precedence relationships to each other; SQL sidesteps the > question by requiring parentheses. They share a set of precedence > relationships to other constructs. SQL does not imply whether to put them in > one %nonassoc precedence group or in a few, but we can contemplate whether > users prefer an error or prefer the 9.4 behavior for affected queries. Part of my thinking was that the 9.4 behavior fails the principle of least astonishment, because I seriously doubt that people expect '=' to be either right-associative or lower priority than '<'. Here's one example: regression=# select false = true < false; ?column? -- t (1 row) Not only does that seem unintuitive, but I actually had to experiment a bit before finding a combination of values in which I got a different result from what you'd expect if you think the precedence is (x = y) < z. So it's not hard to imagine that somebody might write a query thinking that that's how it works, and even have it get through desultory testing before silently giving unexpected answers in production. So yeah, I do think that getting a syntax error if you don't use parentheses is the preferable behavior here. 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] Precedence of standard comparison operators
On Sun, Aug 09, 2015 at 07:16:02PM -0400, Tom Lane wrote: > Noah Misch writes: > > On Sun, Aug 09, 2015 at 06:44:58PM -0400, Tom Lane wrote: > >> So for our > >> purposes, it's better to keep BETWEEN and friends as binding slightly > >> tighter than '<' than to make them the same precedence. Same precedence > >> risks breaking things that weren't broken before. > > > It does risk that. Same deal with making "=" have the same precedence as > > "<" > > instead of keeping it slightly lower. > > Agreed, but in that case I think our hand is forced by the SQL standard. In SQL:2008 and SQL:2011 at least, "=", "<" and "BETWEEN" are all in the same boat. They have no precedence relationships to each other; SQL sidesteps the question by requiring parentheses. They share a set of precedence relationships to other constructs. SQL does not imply whether to put them in one %nonassoc precedence group or in a few, but we can contemplate whether users prefer an error or prefer the 9.4 behavior for affected queries. -- 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] Precedence of standard comparison operators
Noah Misch writes: > On Sun, Aug 09, 2015 at 06:44:58PM -0400, Tom Lane wrote: >> So for our >> purposes, it's better to keep BETWEEN and friends as binding slightly >> tighter than '<' than to make them the same precedence. Same precedence >> risks breaking things that weren't broken before. > It does risk that. Same deal with making "=" have the same precedence as "<" > instead of keeping it slightly lower. Agreed, but in that case I think our hand is forced by the SQL standard. 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] Precedence of standard comparison operators
On Sun, Aug 09, 2015 at 06:44:58PM -0400, Tom Lane wrote: > Noah Misch writes: > > On Sun, Aug 09, 2015 at 04:48:22PM -0400, Tom Lane wrote: > >> I'm curious about your rationale for claiming that has > >> precedence exactly equal to "<" according to the spec. > > > Both and are in the set of > > productions > > that take arguments and appear only in . > > Passing a production in that set as an argument to a production in that set > > requires parentheses. To restate (non-associative) "precedence equal" more > > pedantically, there exists no conforming query whose interpretation hinges > > on > > the relative precedence of "IS NULL" and "<". > > Ah. So really, according to the spec IS and "<" could have any precedence > relative to each other as long as there is no other construct between. > Works for me. > > > To my knowledge, SQL is agnostic about whether LIKE "is an operator". The > > six > > comparison operators bind looser than syntax like > > "*" and "||", tighter than IS TRUE, and indistinguishable from > > syntax like IS DISTINCT FROM and LIKE. > > "Indistinguishable" in the same sense as above, right? Right. > So for our > purposes, it's better to keep BETWEEN and friends as binding slightly > tighter than '<' than to make them the same precedence. Same precedence > risks breaking things that weren't broken before. It does risk that. Same deal with making "=" have the same precedence as "<" instead of keeping it slightly lower. -- 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] Precedence of standard comparison operators
Noah Misch writes: > On Sun, Aug 09, 2015 at 04:48:22PM -0400, Tom Lane wrote: >> I'm curious about your rationale for claiming that has >> precedence exactly equal to "<" according to the spec. > Both and are in the set of productions > that take arguments and appear only in . > Passing a production in that set as an argument to a production in that set > requires parentheses. To restate (non-associative) "precedence equal" more > pedantically, there exists no conforming query whose interpretation hinges on > the relative precedence of "IS NULL" and "<". Ah. So really, according to the spec IS and "<" could have any precedence relative to each other as long as there is no other construct between. Works for me. > To my knowledge, SQL is agnostic about whether LIKE "is an operator". The six > comparison operators bind looser than syntax like > "*" and "||", tighter than IS TRUE, and indistinguishable from > syntax like IS DISTINCT FROM and LIKE. "Indistinguishable" in the same sense as above, right? So for our purposes, it's better to keep BETWEEN and friends as binding slightly tighter than '<' than to make them the same precedence. Same precedence risks breaking things that weren't broken before. Also, while I argued above that making BETWEEN a potential argument for LIKE wasn't sensible, that's not true for the comparison operators. In particular, boolean '<=' is the only SQL-provided spelling for logical implication. >> OVERLAPS is a special case in that it doesn't really need precedence at >> all: both its arguments are required to be parenthesized. We could >> possibly have removed it from the precedence hierarchy altogether, but >> I didn't bother experimenting with that, just left it alone. But >> because of that, "moving BETWEEN/IN below it" doesn't really change >> anything. > Ah, quite right. SQL OVERLAPS takes various forms of two-column input, but > PostgreSQL OVERLAPS is more particular. I like your subsequent proposal to > remove OVERLAPS from the order of precedence. Yeah. If we ever extend our OVERLAPS syntax, we might have to assign a precedence to OVERLAPS, but we can do that then --- and it would be good if we did not at that time have a false impression that the precedence was already determined by previous decisions. I'll go make that change. 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] Precedence of standard comparison operators
On Sun, Aug 09, 2015 at 04:48:22PM -0400, Tom Lane wrote:
> Noah Misch writes:
> > SQL has two groups of IS tests with different precedence. The > test>
> > productions IS [NOT] {TRUE | FALSE | UNKNOWN} have precedence just lower
> > than
> > "<", and the productions IS [NOT] NULL have precedence
> > equal
> > to "<". (An implementation giving them the same precedence can conform,
> > because conforming queries cannot notice the difference.)
>
> I'm curious about your rationale for claiming that has
> precedence exactly equal to "<" according to the spec. AFAICS the SQL
> spec doesn't really tell us much about precedence of different subparts
> of the grammar; at best you can infer that some things bind tighter than
> others.
Both and are in the set of productions
that take arguments and appear only in .
Passing a production in that set as an argument to a production in that set
requires parentheses. To restate (non-associative) "precedence equal" more
pedantically, there exists no conforming query whose interpretation hinges on
the relative precedence of "IS NULL" and "<".
> > 2. Increase precedence of, for example, "BETWEEN x AND Y" to match
> > precedence
> >with "BETWEEN" keyword instead of "AND" keyword. Make similar precedence
> >changes to other multiple-keyword productions involving "AND", "NOT",
> > etc.
>
> Uh, no, I wouldn't have said that. I decreased BETWEEN's precedence,
> along with IN's, to be less than OVERLAPS' precedence, matching the
> precedence of LIKE/ILIKE/SIMILAR. (But see comment below about OVERLAPS.)
> There was not any case where the AND would have determined its precedence
> AFAICS.
Ah. I read this patch hunk carelessly:
> > > @@ -11420,7 +11436,7 @@ a_expr: c_expr
> > > { $$ = $1; }
> > > {
> > > $$ = (Node *)
> > > makeSimpleA_Expr(AEXPR_OF, "<>", $1, (Node *) $6, @2);
> > > }
> > > - | a_expr BETWEEN opt_asymmetric b_expr AND b_expr
> > > %prec BETWEEN
> > > + | a_expr BETWEEN opt_asymmetric b_expr AND a_expr
> > > %prec BETWEEN
That's allowing additional productions in the final BETWEEN operand, not
changing precedence.
> > 5. Forbid chains of "=" (make it nonassoc), and increase its precedence to
> >match "<".
> > 6. Decrease precedence of BETWEEN and IN keywords to match "LIKE".
> > I've been unable to explain (5) and (6).
>
> I'm not following your concern about (5). The spec seems to clearly
> put all six basic comparison operators on the same precedence level.
> [snipped rest of explanation]
No particular concern beyond wanting to know the rationale; thanks for writing
one. Getting this wrong a second time would be awfully sad, so I'm being more
cautious than usual.
> > Why in particular the following three precedence groups instead of
> > combining them as in SQL or subdividing further as in PostgreSQL 9.4?
>
> >> +%nonassoc '<' '>' '=' LESS_EQUALS GREATER_EQUALS NOT_EQUALS
> >> +%nonassoc BETWEEN IN_P LIKE ILIKE SIMILAR NOT_LA
> >> %nonassoc OVERLAPS
>
> I think that the spec is fairly clear that the six comparison operators
> bind looser than other operators. Now you could argue about whether LIKE
> et al are "operators" but Postgres certainly treats them as such.
To my knowledge, SQL is agnostic about whether LIKE "is an operator". The six
comparison operators bind looser than syntax like
"*" and "||", tighter than IS TRUE, and indistinguishable from
syntax like IS DISTINCT FROM and LIKE.
> OVERLAPS is a special case in that it doesn't really need precedence at
> all: both its arguments are required to be parenthesized. We could
> possibly have removed it from the precedence hierarchy altogether, but
> I didn't bother experimenting with that, just left it alone. But
> because of that, "moving BETWEEN/IN below it" doesn't really change
> anything.
Ah, quite right. SQL OVERLAPS takes various forms of two-column input, but
PostgreSQL OVERLAPS is more particular. I like your subsequent proposal to
remove OVERLAPS from the order of precedence.
Thanks,
nm
--
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] Precedence of standard comparison operators
I wrote: > Noah Misch writes: >> Why in particular the following three precedence groups instead of >> combining them as in SQL or subdividing further as in PostgreSQL 9.4? > +%nonassoc'<' '>' '=' LESS_EQUALS GREATER_EQUALS NOT_EQUALS > +%nonassocBETWEEN IN_P LIKE ILIKE SIMILAR NOT_LA > %nonassoc OVERLAPS > OVERLAPS is a special case in that it doesn't really need precedence at > all: both its arguments are required to be parenthesized. We could > possibly have removed it from the precedence hierarchy altogether, but > I didn't bother experimenting with that, just left it alone. But > because of that, "moving BETWEEN/IN below it" doesn't really change > anything. I got off my rear and did the experiment, and found that indeed simply removing "%nonassoc OVERLAPS" seems to work and not change any behavior (in fact, the generated gram.c is identical). Shall we do that and remove the listing of OVERLAPS in the documentation's precedence table? 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] Precedence of standard comparison operators
Noah Misch writes:
> SQL has two groups of IS tests with different precedence. The
> productions IS [NOT] {TRUE | FALSE | UNKNOWN} have precedence just lower than
> "<", and the productions IS [NOT] NULL have precedence equal
> to "<". (An implementation giving them the same precedence can conform,
> because conforming queries cannot notice the difference.)
I'm curious about your rationale for claiming that has
precedence exactly equal to "<" according to the spec. AFAICS the SQL
spec doesn't really tell us much about precedence of different subparts
of the grammar; at best you can infer that some things bind tighter than
others.
> I attempted to catalog the diverse precedence changes in commit c6b3c93:
> 1. Decrease precedence of "<=", ">=" and "<>" to match "<".
Check.
> 2. Increase precedence of, for example, "BETWEEN x AND Y" to match precedence
>with "BETWEEN" keyword instead of "AND" keyword. Make similar precedence
>changes to other multiple-keyword productions involving "AND", "NOT", etc.
Uh, no, I wouldn't have said that. I decreased BETWEEN's precedence,
along with IN's, to be less than OVERLAPS' precedence, matching the
precedence of LIKE/ILIKE/SIMILAR. (But see comment below about OVERLAPS.)
There was not any case where the AND would have determined its precedence
AFAICS.
> 3. Decrease precedence of IS [NOT] {TRUE | FALSE | UNKNOWN} to fall between
>NOT and "<".
Check.
> 4. Decrease precedence of IS [NOT] NULL and IS[NOT]NULL to match IS [NOT]
>{TRUE | FALSE | UNKNOWN}.
Check.
> 5. Forbid chains of "=" (make it nonassoc), and increase its precedence to
>match "<".
Check.
> 6. Decrease precedence of BETWEEN and IN keywords to match "LIKE".
Check.
>> It's
>> definitely weird that the IS tests bind more tightly than multicharacter
>> Ops but less tightly than + - * /.
> (1), (2) and (3) improve SQL conformance, and that last sentence seems to
> explain your rationale for (4).
The real reason for (4) is that it would be very difficult to get bison to
consider IS TRUE to have different precedence (against an operator on its
left) than IS NULL; we'd need even more lookahead mess than we have now.
They were not different precedence before, and they aren't now.
I did change ISNULL and NOTNULL to have exactly the same precedence as the
long forms IS NULL and IS NOT NULL, where before they were (for no good
reason AFAICS) slightly different precedence. I think that's justifiable
on the grounds that they should not act differently from the long forms.
In any case the SQL standard has nothing to teach us on the point, since
it doesn't admit these shorthands.
> I've been unable to explain (5) and (6).
I'm not following your concern about (5). The spec seems to clearly
put all six basic comparison operators on the same precedence level.
I believe that the reason our grammar had '=' as right-associative is
someone's idea that we might someday consider assignment as a regular
operator a la C, but that never has been true and seems unlikely to
become true in the future. There's certainly nothing in the spec
suggesting that '=' should be right-associative.
The reason for (6) was mainly that having IN/BETWEEN bind tighter than
LIKE doesn't seem to me to have any justification in the spec; moreover,
if it does bind tighter, we're delivering a boolean result to LIKE which
seems unlikely to be what anyone wants. So I figured we could simplify
things a bit by having one (or two really) fewer precedence levels.
Also, because said level is %nonassoc, this will now force people to
parenthesize if they do indeed want something like a BETWEEN as argument
of a LIKE, which seems like a good thing all round.
> Why in particular the following three precedence groups instead of
> combining them as in SQL or subdividing further as in PostgreSQL 9.4?
>> +%nonassoc '<' '>' '=' LESS_EQUALS GREATER_EQUALS NOT_EQUALS
>> +%nonassoc BETWEEN IN_P LIKE ILIKE SIMILAR NOT_LA
>> %nonassocOVERLAPS
I think that the spec is fairly clear that the six comparison operators
bind looser than other operators. Now you could argue about whether LIKE
et al are "operators" but Postgres certainly treats them as such.
OVERLAPS is a special case in that it doesn't really need precedence at
all: both its arguments are required to be parenthesized. We could
possibly have removed it from the precedence hierarchy altogether, but
I didn't bother experimenting with that, just left it alone. But
because of that, "moving BETWEEN/IN below it" doesn't really change
anything.
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] Precedence of standard comparison operators
On Thu, Feb 19, 2015 at 10:48:34AM -0500, Tom Lane wrote:
> To wit, that the precedence of <= >= and <> is neither sane nor standards
> compliant.
> I claim that this behavior is contrary to spec as well as being
> unintuitive. Following the grammar productions in SQL99:
Between 1999 and 2006, SQL changed its representation of the grammar in this
area; I have appended to this message some of the key productions as of 2013.
I did not notice a semantic change, though.
> We have that right for = < > but not for the other three standard-mandated
> comparison operators. I think we should change the grammar so that all
> six act like < > do now, that is, they should have %nonassoc precedence
> just above NOT.
>
> Another thought, looking at this closely, is that we have the precedence
> of IS tests (IS NOT NULL etc) wrong as well: they should bind less tightly
> than user-defined ops, not more so.
SQL has two groups of IS tests with different precedence. The
productions IS [NOT] {TRUE | FALSE | UNKNOWN} have precedence just lower than
"<", and the productions IS [NOT] NULL have precedence equal
to "<". (An implementation giving them the same precedence can conform,
because conforming queries cannot notice the difference.)
I attempted to catalog the diverse precedence changes in commit c6b3c93:
> @@ -647,13 +654,11 @@ static Node *makeRecursiveViewSelect(char *relname,
> List *aliases, Node *query);
> %leftOR
> %leftAND
> %right NOT
> -%right '='
> -%nonassoc'<' '>'
> -%nonassocLIKE ILIKE SIMILAR
> -%nonassocESCAPE
> +%nonassocIS ISNULL NOTNULL /* IS sets precedence for IS NULL, etc
> */
> +%nonassoc'<' '>' '=' LESS_EQUALS GREATER_EQUALS NOT_EQUALS
> +%nonassocBETWEEN IN_P LIKE ILIKE SIMILAR NOT_LA
> +%nonassocESCAPE /* ESCAPE must be just above
> LIKE/ILIKE/SIMILAR */
> %nonassocOVERLAPS
> -%nonassocBETWEEN
> -%nonassocIN_P
> %leftPOSTFIXOP /* dummy for postfix Op rules */
> /*
> * To support target_el without AS, we must give IDENT an explicit priority
> @@ -678,9 +683,6 @@ static Node *makeRecursiveViewSelect(char *relname, List
> *aliases, Node *query);
> %nonassocUNBOUNDED /* ideally should have same precedence
> as IDENT */
> %nonassocIDENT NULL_P PARTITION RANGE ROWS PRECEDING FOLLOWING
> %leftOp OPERATOR /* multi-character ops and
> user-defined operators */
> -%nonassocNOTNULL
> -%nonassocISNULL
> -%nonassocIS /* sets precedence for IS NULL,
> etc */
> %left'+' '-'
> %left'*' '/' '%'
> %left'^'
1. Decrease precedence of "<=", ">=" and "<>" to match "<".
2. Increase precedence of, for example, "BETWEEN x AND Y" to match precedence
with "BETWEEN" keyword instead of "AND" keyword. Make similar precedence
changes to other multiple-keyword productions involving "AND", "NOT", etc.
3. Decrease precedence of IS [NOT] {TRUE | FALSE | UNKNOWN} to fall between
NOT and "<".
4. Decrease precedence of IS [NOT] NULL and IS[NOT]NULL to match IS [NOT]
{TRUE | FALSE | UNKNOWN}.
5. Forbid chains of "=" (make it nonassoc), and increase its precedence to
match "<".
6. Decrease precedence of BETWEEN and IN keywords to match "LIKE".
> It's
> definitely weird that the IS tests bind more tightly than multicharacter
> Ops but less tightly than + - * /.
(1), (2) and (3) improve SQL conformance, and that last sentence seems to
explain your rationale for (4). I've been unable to explain (5) and (6). Why
in particular the following three precedence groups instead of combining them
as in SQL or subdividing further as in PostgreSQL 9.4?
> +%nonassoc'<' '>' '=' LESS_EQUALS GREATER_EQUALS NOT_EQUALS
> +%nonassocBETWEEN IN_P LIKE ILIKE SIMILAR NOT_LA
> %nonassocOVERLAPS
Thanks,
nm
::=
::=
::=
|
# Syntax Rules
# 1) The declared type of a shall be a row type.
::=
# Things with precedence higher than comparison.
::=
|
|
# numeric: addition, multiplication
# string: concat, collate clause
# datetime: addition, AT TIME ZONE
# interval: addition, division
# UDT:
# reference:
# collection: array/multiset
::=
|
|
|
|
|
|
::=
|
::=
# Things unambiguous without parens.
::=
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
::=
| OR
::=
| AND
::=
[ NOT ]
::=
[ IS [ NOT ] ]
::=
|
::=
TRUE
| FALSE
| UNKNOWN
# Things with precedence equal to comparison.
::=
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
::=
::=
IS [ NOT ] NULL
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http:/
Re: [HACKERS] Precedence of standard comparison operators
On Tue, Mar 10, 2015 at 04:10:01PM -0400, Peter Eisentraut wrote: > On 3/10/15 1:12 PM, Tom Lane wrote: > > Robert Haas writes: > >> On Tue, Mar 10, 2015 at 12:45 PM, David G. Johnston > >> wrote: > >>> I would vote for Auto meaning On in the .0 release. > > > >> I don't think users will appreciate an auto value whose meaning might > >> change at some point, and I doubt we've have much luck identifying the > >> correct point, either. Users will upgrade over the course of years, > >> not months, and will not necessarily complete their application > >> retesting within any particular period of time thereafter. > > > > Yeah, I think that's too cute by far. And people do not like things like > > this changing in point releases. If we do this, I envision it as being > > on by default in 9.5 and then changing the default in 9.6 or 9.7 or so. > > Well, I point again to standards_conforming_strings: Leave the warning > off for one release (or more), then default to on for one (or more), > then change the behavior. > > We can change the timeline, but I don't think the approach was unsound. Sorry to be replying late (but when has that ever stopped me :-) ), but for standards_conforming_strings, there were security concerns about the change, and we are forcing people to move to a new necessary syntax when using backslash escapes, i.e. E''. Once they moved to E'', the warnings went away. In this case, the way to avoid the warnings is to add _unnecessary_ parentheses, and sometimes in cases that don't need them, and never will. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Precedence of standard comparison operators
On Wed, Mar 11, 2015 at 8:36 PM, Kevin Grittner wrote: > Right; and they may have millions of lines of code which have been > carefully tested and in production for years, and which may > suddenly start to generate incorrect results on queries *or mangle > existing data* with the fix unless they change their SQL code. > Well not suddenly. When doing a major upgrade. And we provide a tool to help them identify the problems. But having a warning enabled by default means the problem is effectively not fixed at all. People will not be able to write code that's valid according to the docs and the spec without extra parentheses or disabling the warning. -- greg
Re: [HACKERS] Precedence of standard comparison operators
On 3/10/15 4:34 PM, Tom Lane wrote: > There's one more reason, too: the code I have is designed to give correct > warnings within the context of a parser that parses according to the > spec-compliant rules. It's possible that a similar approach could be used > to generate correct warnings within a parsetree built according to the old > rules, but it would be entirely different in detail and would need a lot > of additional work to develop. I don't particularly want to do that > additional work. So you want to change the precedence behavior in the next release and have a warning about "this code would have worked differently before". My understanding was that we would keep the precedence behavior for a while but warn about "this code would work differently in the future". -- 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] Precedence of standard comparison operators
Robert Haas writes: > On Tue, Mar 10, 2015 at 12:45 PM, David G. Johnston > wrote: >> I would vote for Auto meaning On in the .0 release. > I don't think users will appreciate an auto value whose meaning might > change at some point, and I doubt we've have much luck identifying the > correct point, either. Users will upgrade over the course of years, > not months, and will not necessarily complete their application > retesting within any particular period of time thereafter. Yeah, I think that's too cute by far. And people do not like things like this changing in point releases. If we do this, I envision it as being on by default in 9.5 and then changing the default in 9.6 or 9.7 or so. Another possibility is to leave it on through beta testing with the intent to turn it off before 9.5 final; that would give us more data about whether there are real issues than we're likely to get otherwise. 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] Precedence of standard comparison operators
Robert Haas writes: > Just out of curiosity, does this change create a dump-and-reload > hazard? Like if I pg_upgrade my cluster, will the output of pg_dump > potentially be sufficiently under-parenthesized that reload will > create a non-equivalent database? No. Had there been any such hazard, I would not have been nearly as enthused about this project. In the first place, pg_dump disables ruleutils' pretty-printing heuristics, so that you always get fully parenthesized expressions no matter what. I insisted on that years ago, in the expectation that we might someday need to do things like this patch; it's always acted that way, in every release that had any pretty-printing ability at all. For example, regression=# create view vv as select unique1 + unique2*four as x from tenk1; CREATE VIEW regression=# \d+ vv View "public.vv" Column | Type | Modifiers | Storage | Description +-+---+-+- x | integer | | plain | View definition: SELECT tenk1.unique1 + tenk1.unique2 * tenk1.four AS x FROM tenk1; regression=# \q $ pg_dump -t vv regression ... CREATE VIEW vv AS SELECT (tenk1.unique1 + (tenk1.unique2 * tenk1.four)) AS x FROM tenk1; In the second place, AFAICS ruleutils' heuristics do not know anything about any of the constructs affected by this patch, and so they'll be fully parenthesized anyway, even in pretty-printed output. For example: regression=# create view vvv as select 1+2 is distinct from 42 as q; CREATE VIEW regression=# \d+ vvv View "public.vvv" Column | Type | Modifiers | Storage | Description +-+---+-+- q | boolean | | plain | View definition: SELECT (1 + 2) IS DISTINCT FROM 42 AS q; The parens are unnecessary, but ruleutils doesn't know that, so it puts them in. 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] Precedence of standard comparison operators
On Wed, Mar 11, 2015 at 7:49 PM, Tom Lane wrote: > Kevin Grittner writes: >> If there are no false positives, turning it on is zero impact >> (except for any performance impact involved in detecting the >> condition) for those who have no problems. That will probably be >> the vast majority of users. The question is, do we want to quietly >> do something new and different for the small percentage of users >> who will have a problem, and leave it to them to find out about >> this setting and turn on the feature that tells them where the >> problems are? Or would it be more friendly to show the issues so >> they can resolve them, and then turn off the warnings once they are >> satisfied? > > FWIW, there *are* some especially-corner-casey false positives, > as I noted upthread. I think the odds of people hitting them > are remarkably low, which is why I wasn't willing to invest the > additional code needed to try to make them all go away. I doubt > that this consideration is worth worrying about as we decide > whether the warnings should be on by default ... but if you're > going to make an argument from an assumption of no false positives, > it's wrong. Just out of curiosity, does this change create a dump-and-reload hazard? Like if I pg_upgrade my cluster, will the output of pg_dump potentially be sufficiently under-parenthesized that reload will create a non-equivalent database? -- Robert Haas EnterpriseDB: 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] Precedence of standard comparison operators
On Wed, Mar 11, 2015 at 6:19 PM, Kevin Grittner wrote: > Either way it is like leaving the barn door open so that horses are > capable of running out. We have an alarm that lets you know when > something is going through the barn door; the question is whether > to default that alarm on or off. What we have an alarm that lets you know that your perfectly legitimate code might have meant something different in a release you aren't running. If that helps you catch a bug in code you are porting from a previous version, great. But otherwise it's simply a nuisance. I don't think this is like leaving the barn door open so that horses are capable of running out. I think it's more like insisting on wiring an alarm to every barn door in the county whether there are any animals currently housed in that barn or not. Now there is nothing wrong with giving away free alarm systems, but insisting on turning them all on except for the people who explicitly turn them off seems a little pushy. Also, given Tom's remarks downthread, we seem to still lack a plausible use case where the same code is legal in both versions but works differently. We should really keep trying to find one of those, because I think it would shed a lot of light on this debate. -- Robert Haas EnterpriseDB: 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] Precedence of standard comparison operators
Kevin Grittner writes: > If there are no false positives, turning it on is zero impact > (except for any performance impact involved in detecting the > condition) for those who have no problems. That will probably be > the vast majority of users. The question is, do we want to quietly > do something new and different for the small percentage of users > who will have a problem, and leave it to them to find out about > this setting and turn on the feature that tells them where the > problems are? Or would it be more friendly to show the issues so > they can resolve them, and then turn off the warnings once they are > satisfied? FWIW, there *are* some especially-corner-casey false positives, as I noted upthread. I think the odds of people hitting them are remarkably low, which is why I wasn't willing to invest the additional code needed to try to make them all go away. I doubt that this consideration is worth worrying about as we decide whether the warnings should be on by default ... but if you're going to make an argument from an assumption of no false positives, it's 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] Precedence of standard comparison operators
Kevin Grittner writes: > Robert Haas wrote: >> Can you, or can anyone, show a plausible example of something >> that would work under the old rules and work under the new rules >> but with a different meaning? I have to admit that I'm having >> some difficulty imagining exactly when that happens. Tom's >> examples upthread were not things that seemed all that likely. > I think very few will see any problems. At Wisconsin Courts we had > millions of lines of code when we moved to PostgreSQL and saw one > or two queries which had problems with this. The example I gave > earlier on this thread was: > | test=# select 'f'::boolean = 'f'::boolean >= 'f'::boolean; > | ?column? > | -- > | f > | (1 row) > Now, that was a simple case for purposes of illustration, but > imagine that the first two literals are column names in a complex > query and it becomes slightly less ridiculous. Imagine they are > being passed in to some plpgsql function which accepts different > types, and the statement is run through EXECUTE and it may be > somewhat reasonable. Note that that example now *fails*, because = and => now belong to the same precedence level which is %nonassoc: regression=# select 'f'::boolean = 'f'::boolean >= 'f'::boolean; ERROR: syntax error at or near ">=" LINE 1: select 'f'::boolean = 'f'::boolean >= 'f'::boolean; ^ (It'd be nice if it gave a more specific error message than just "syntax error", but I'm not sure if there's any practical way to persuade bison to do something different than that.) I think that the set of practical examples that produce a different answer without outright failing (either due to %nonassoc or due to lack of any matching operator) is really going to be darn small. > Either way it is like leaving the barn door open so that horses are > capable of running out. We have an alarm that lets you know when > something is going through the barn door; the question is whether > to default that alarm on or off. If we believe that the set of affected apps really is small, then it seems like there should not be much downside to having the warning on by default: by hypothesis, few people will ever see it at all. OTOH, there is some small run-time cost to having it on... The %nonassoc syntax error problem is likely to be a bigger problem for user-friendliness than anything else about this patch, IMO. Now that we're committed to this course, I will do some research and see if that behavior can be improved at all. 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] Precedence of standard comparison operators
Robert Haas wrote: > Can you, or can anyone, show a plausible example of something > that would work under the old rules and work under the new rules > but with a different meaning? I have to admit that I'm having > some difficulty imagining exactly when that happens. Tom's > examples upthread were not things that seemed all that likely. I think very few will see any problems. At Wisconsin Courts we had millions of lines of code when we moved to PostgreSQL and saw one or two queries which had problems with this. The example I gave earlier on this thread was: | test=# select 'f'::boolean = 'f'::boolean >= 'f'::boolean; | ?column? | -- | f | (1 row) Now, that was a simple case for purposes of illustration, but imagine that the first two literals are column names in a complex query and it becomes slightly less ridiculous. Imagine they are being passed in to some plpgsql function which accepts different types, and the statement is run through EXECUTE and it may be somewhat reasonable. The upshot is that for at least 99% of shops turning this on will not generate any warnings at all. So, the question is what is better for the cases where it will actually matter? Either way it is like leaving the barn door open so that horses are capable of running out. We have an alarm that lets you know when something is going through the barn door; the question is whether to default that alarm on or off. -- 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] Precedence of standard comparison operators
On Wed, Mar 11, 2015 at 4:36 PM, Kevin Grittner wrote: > If we ship with this off the results are entirely predictable. It > will be somewhat surprising not to see any negative headlines about > it. Can you, or can anyone, show a plausible example of something that would work under the old rules and work under the new rules but with a different meaning? I have to admit that I'm having some difficulty imagining exactly when that happens. Tom's examples upthread were not things that seemed all that likely. The most plausible example was probably a <= b || c, but the *old* interpretation of that is (a <= b) || c, so I'm having a little trouble taking that seriously as an example of where this would cause a problem. If the old interpretation had been a <= (b || c) and we were changing that to (a <= b) || c, then, yeah, that could break things for a lot of people, but not so many in this direction. Are there better examples of how this is going to be bite people? I really don't want to have another implicit-casting-changes type debacle here. -- Robert Haas EnterpriseDB: 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] Precedence of standard comparison operators
Greg Stark wrote: > On Wed, Mar 11, 2015 at 8:00 PM, Kevin Grittner wrote: >> If there are no false positives, turning it on is zero impact >> (except for any performance impact involved in detecting the >> condition) for those who have no problems. > Think of this as a bug fix. I do. :-) > Hopefully nobody was using the syntax before because it didn't > work right and if it did they were depending on the broken > behaviour. Right; and they may have millions of lines of code which have been carefully tested and in production for years, and which may suddenly start to generate incorrect results on queries *or mangle existing data* with the fix unless they change their SQL code. > But once it's fixed it would be frustrating to have it be fixed > but have a warning saying "WARNING this query works fine now but > didn't work in previous versions". That's getting into subjective territory. What you say "works fine" may be completely unintended an unexpected, and may cause serious disruption of their business. You seem to be arguing that they deserve it for counting on our old, buggy implementation, and coming to accept that as expected behavior. > Especially since warnings in DML are effectively fatal errors due > to the frequency that they'll fire. In what situation? Code which was running under a previous major release of PostgreSQL? If they are getting so many of these warnings in that case, they would probably prefer to have this caught in pre-upgrade testing or at least as soon as possible. If it is new code written under 9.5 one would hope they have a testing phase where this would be detected so they could choose to add the parentheses needed to maintain portable code or turn off the warning. > The warning can be useful for people testing code written in old > versions or writing code intended to be multi-version compatible. We agree there... > But it's not something someone would want if they're writing code > for 9.5. ... and we also agree here; but I'm saying that in that case it will still rarely come up and if it does it's easy enough to disable. That inconvenience is not sufficient to not want to, by default, mangle the data or query results of some small percentage of existing users. If we ship with this off the results are entirely predictable. It will be somewhat surprising not to see any negative headlines about it. -- 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] Precedence of standard comparison operators
On Wed, Mar 11, 2015 at 8:00 PM, Kevin Grittner wrote: > If there are no false positives, turning it on is zero impact > (except for any performance impact involved in detecting the > condition) for those who have no problems. > Think of this as a bug fix. Hopefully nobody was using the syntax before because it didn't work right and if it did they were depending on the broken behaviour. But once it's fixed it would be frustrating to have it be fixed but have a warning saying "WARNING this query works fine now but didn't work in previous versions". Especially since warnings in DML are effectively fatal errors due to the frequency that they'll fire. The warning can be useful for people testing code written in old versions or writing code intended to be multi-version compatible. But it's not something someone would want if they're writing code for 9.5. -- greg
Re: [HACKERS] Precedence of standard comparison operators
Tom Lane wrote: > I wrote: >> Robert Haas writes: >>> On Tue, Mar 10, 2015 at 1:12 PM, Tom Lane wrote: Another possibility is to leave it on through beta testing with the intent to turn it off before 9.5 final; that would give us more data about whether there are real issues than we're likely to get otherwise. >>> >>> To my mind, the fact that we're doing this at all is largely >>> predicated on the fact that there won't be many real issues. >>> So I think the goal of the debugging messages ought to be to >>> let those people who discover that they do have issues track >>> them down more easily, not to warn people. Warning is sort of >>> closing the barn door after the horse has got out: hey, by the >>> way, I just broke your app. I don't get this argument at all. If the new code doesn't cause your queries to return different results (or update or delete different rows), you don't get a warning, right? Or are there false positives? If there are no false positives, turning it on is zero impact (except for any performance impact involved in detecting the condition) for those who have no problems. That will probably be the vast majority of users. The question is, do we want to quietly do something new and different for the small percentage of users who will have a problem, and leave it to them to find out about this setting and turn on the feature that tells them where the problems are? Or would it be more friendly to show the issues so they can resolve them, and then turn off the warnings once they are satisfied? Once users have established that they have no code that has different semantics under the standard operator precedence, they may want to turn it off to avoid the need for extra parentheses where pre-9.5 behavior would be different; but unless there is a significant performance hit for the check, they may want to leave it on just as insurance against mistakes. > Given the lack of enthusiasm about that argument, I've pushed the > patch with warnings off by default. It's certainly easy enough > to change that aspect later if we choose to. I think it should default to on for several major releases. Once the standard behavior is well-established we can default to off. I see that as a three to five year interval. -- 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] Precedence of standard comparison operators
I wrote: > Robert Haas writes: >> On Tue, Mar 10, 2015 at 1:12 PM, Tom Lane wrote: >>> Another possibility is to leave it on through beta testing with the intent >>> to turn it off before 9.5 final; that would give us more data about >>> whether there are real issues than we're likely to get otherwise. >> To my mind, the fact that we're doing this at all is largely >> predicated on the fact that there won't be many real issues. So I >> think the goal of the debugging messages ought to be to let those >> people who discover that they do have issues track them down more >> easily, not to warn people. Warning is sort of closing the barn door >> after the horse has got out: hey, by the way, I just broke your app. > Agreed, but in the near term we need to *find out* whether there will > be many real issues. Perhaps having the warnings on by default would > help that, or perhaps not; I'm not sure. Given the lack of enthusiasm about that argument, I've pushed the patch with warnings off by default. It's certainly easy enough to change that aspect later if we choose to. 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] Precedence of standard comparison operators
Peter Eisentraut writes: > Well, I point again to standards_conforming_strings: Leave the warning > off for one release (or more), then default to on for one (or more), > then change the behavior. > We can change the timeline, but I don't think the approach was unsound. I'm not excited about that approach, for the reasons that were stated upthread, mainly that there is little reason to think that anybody paid any attention to the s_c_s transition till they were forced to. Waiting to make the change will just allow more non-spec-compliant SQL code to accumulate in the wild, without significantly reducing the pain involved. There's one more reason, too: the code I have is designed to give correct warnings within the context of a parser that parses according to the spec-compliant rules. It's possible that a similar approach could be used to generate correct warnings within a parsetree built according to the old rules, but it would be entirely different in detail and would need a lot of additional work to develop. I don't particularly want to do that additional work. 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] Precedence of standard comparison operators
On 3/10/15 1:12 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Mar 10, 2015 at 12:45 PM, David G. Johnston >> wrote: >>> I would vote for Auto meaning On in the .0 release. > >> I don't think users will appreciate an auto value whose meaning might >> change at some point, and I doubt we've have much luck identifying the >> correct point, either. Users will upgrade over the course of years, >> not months, and will not necessarily complete their application >> retesting within any particular period of time thereafter. > > Yeah, I think that's too cute by far. And people do not like things like > this changing in point releases. If we do this, I envision it as being > on by default in 9.5 and then changing the default in 9.6 or 9.7 or so. Well, I point again to standards_conforming_strings: Leave the warning off for one release (or more), then default to on for one (or more), then change the behavior. We can change the timeline, but I don't think the approach was unsound. -- 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] Precedence of standard comparison operators
Alex Hunsaker writes: > On Tue, Mar 10, 2015 at 10:11 AM, Tom Lane wrote: >> This thread seems to have died off without any clear resolution. I'd >> hoped somebody would try the patch on some nontrivial application to >> see if it broke anything or caused any warnings, but it doesn't seem >> like that is happening. > I tried it on a fairly typical web application. Around 5000 or so distinct > statements according to pg_stat_statements. With > operator_precedence_warning = on, no warnings yet. Thanks! Much appreciated. 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] Precedence of standard comparison operators
On Tue, Mar 10, 2015 at 10:11 AM, Tom Lane wrote: > I wrote: > > This thread seems to have died off without any clear resolution. I'd > hoped somebody would try the patch on some nontrivial application to > see if it broke anything or caused any warnings, but it doesn't seem > like that is happening. > > I tried it on a fairly typical web application. Around 5000 or so distinct statements according to pg_stat_statements. With operator_precedence_warning = on, no warnings yet.
Re: [HACKERS] Precedence of standard comparison operators
Robert Haas writes: > On Tue, Mar 10, 2015 at 1:12 PM, Tom Lane wrote: >> Another possibility is to leave it on through beta testing with the intent >> to turn it off before 9.5 final; that would give us more data about >> whether there are real issues than we're likely to get otherwise. > To my mind, the fact that we're doing this at all is largely > predicated on the fact that there won't be many real issues. So I > think the goal of the debugging messages ought to be to let those > people who discover that they do have issues track them down more > easily, not to warn people. Warning is sort of closing the barn door > after the horse has got out: hey, by the way, I just broke your app. Agreed, but in the near term we need to *find out* whether there will be many real issues. Perhaps having the warnings on by default would help that, or perhaps not; I'm not sure. > Another thing to consider is that if it becomes common to run with > these warnings on, then everybody will have to pretty much write their > code with full parenthesization anyway, at least if they plan to > publish their code on PGXN or anywhere that it might get run on some > system other than the one it was written for. That seems like an > annoying gotcha for an issue that we're not expecting to be common. Hm, well, people who are publishing code will likely want it to work on both old and new PG releases, so I suspect they'd need to make it run warning-free anyway. 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] Precedence of standard comparison operators
On Tue, Mar 10, 2015 at 1:12 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Mar 10, 2015 at 12:45 PM, David G. Johnston >> wrote: >>> I would vote for Auto meaning On in the .0 release. > >> I don't think users will appreciate an auto value whose meaning might >> change at some point, and I doubt we've have much luck identifying the >> correct point, either. Users will upgrade over the course of years, >> not months, and will not necessarily complete their application >> retesting within any particular period of time thereafter. > > Yeah, I think that's too cute by far. And people do not like things like > this changing in point releases. If we do this, I envision it as being > on by default in 9.5 and then changing the default in 9.6 or 9.7 or so. > > Another possibility is to leave it on through beta testing with the intent > to turn it off before 9.5 final; that would give us more data about > whether there are real issues than we're likely to get otherwise. To my mind, the fact that we're doing this at all is largely predicated on the fact that there won't be many real issues. So I think the goal of the debugging messages ought to be to let those people who discover that they do have issues track them down more easily, not to warn people. Warning is sort of closing the barn door after the horse has got out: hey, by the way, I just broke your app. Another thing to consider is that if it becomes common to run with these warnings on, then everybody will have to pretty much write their code with full parenthesization anyway, at least if they plan to publish their code on PGXN or anywhere that it might get run on some system other than the one it was written for. That seems like an annoying gotcha for an issue that we're not expecting to be common. -- Robert Haas EnterpriseDB: 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] Precedence of standard comparison operators
On Tue, Mar 10, 2015 at 12:45 PM, David G. Johnston wrote: > On Tue, Mar 10, 2015 at 9:37 AM, Robert Haas wrote: >> On Tue, Mar 10, 2015 at 12:11 PM, Tom Lane wrote: >> > Do we have consensus on doing this? Should we have the warning on >> > by default, or off? >> >> I vote for defaulting the warning to off. If that proves to be too >> problematic, I'd take that as a sign that this whole change is not as >> low-impact as we're hoping, and maybe consider a rethink. > > Do we want to have three states? On, Off, and Auto? We can then change > what Auto means in a point-release while letting people who have chosen On > or Off have their wish. > > Auto could also consider some other data - like how long ago the database > was initialized... > > I would vote for Auto meaning On in the .0 release. I don't think users will appreciate an auto value whose meaning might change at some point, and I doubt we've have much luck identifying the correct point, either. Users will upgrade over the course of years, not months, and will not necessarily complete their application retesting within any particular period of time thereafter. -- Robert Haas EnterpriseDB: 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] Precedence of standard comparison operators
On Tue, Mar 10, 2015 at 9:37 AM, Robert Haas wrote: > On Tue, Mar 10, 2015 at 12:11 PM, Tom Lane wrote: > > Do we have consensus on doing this? Should we have the warning on > > by default, or off? > > I vote for defaulting the warning to off. If that proves to be too > problematic, I'd take that as a sign that this whole change is not as > low-impact as we're hoping, and maybe consider a rethink. > Do we want to have three states? On, Off, and Auto? We can then change what Auto means in a point-release while letting people who have chosen On or Off have their wish. Auto could also consider some other data - like how long ago the database was initialized... I would vote for Auto meaning On in the .0 release. David J.
Re: [HACKERS] Precedence of standard comparison operators
On Tue, Mar 10, 2015 at 12:11 PM, Tom Lane wrote: > I wrote: >> Andres Freund writes: >>> On February 26, 2015 10:29:18 PM CET, Peter Eisentraut >>> wrote: My suggestion was to treat this like the standard_conforming_string change. That is, warn for many years before changing. > >>> I don't think scs is a good example to follow. > >> Yeah. For one thing, there wouldn't be any way to suppress the warning >> other than to parenthesize your code, which I would find problematic >> because it would penalize standard-conforming queries. I'd prefer an >> arrangement whereby once you fix your code to be standard-conforming, >> you're done. > >> A possible point of compromise would be to leave the warning turned on >> by default, at least until we get a better sense of how this would >> play out in the real world. I continue to suspect that we're making >> a mountain out of, if not a molehill, at least a hillock. I think most >> sane people would have parenthesized their queries to start with rather >> than go look up whether IS DISTINCT FROM binds tighter than <= ... > > This thread seems to have died off without any clear resolution. I'd > hoped somebody would try the patch on some nontrivial application to > see if it broke anything or caused any warnings, but it doesn't seem > like that is happening. > > Do we have consensus on doing this? Should we have the warning on > by default, or off? I vote for defaulting the warning to off. If that proves to be too problematic, I'd take that as a sign that this whole change is not as low-impact as we're hoping, and maybe consider a rethink. -- Robert Haas EnterpriseDB: 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] Precedence of standard comparison operators
Tom Lane wrote: > Do we have consensus on doing this? Should we have the warning on > by default, or off? This is the tough decision, isn't it. I had thought it would default to off and people would only turn it on in their testing procedure prior to the actual upgrade of the production systems, but how are they going to find out they need to turn it on in the first place? We could have a big fat red blinking warning in the release notes and a picture of a dancing elephant in a tutu next to it, and we can be certain that many will miss it anyway. I think we should have an "expires" value: it means ON unless the system's initdb is older than one month from the current date, in which case it turns itself off. This is of course just a silly joke, but as you said there are probably valid constructs that are going to raise warnings for no reason, so having it default to ON would be pointlessly noisy in systems that have been developed with the new rules. -- Álvaro Herrerahttp://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] Precedence of standard comparison operators
I wrote: > Andres Freund writes: >> On February 26, 2015 10:29:18 PM CET, Peter Eisentraut >> wrote: >>> My suggestion was to treat this like the standard_conforming_string >>> change. That is, warn for many years before changing. >> I don't think scs is a good example to follow. > Yeah. For one thing, there wouldn't be any way to suppress the warning > other than to parenthesize your code, which I would find problematic > because it would penalize standard-conforming queries. I'd prefer an > arrangement whereby once you fix your code to be standard-conforming, > you're done. > A possible point of compromise would be to leave the warning turned on > by default, at least until we get a better sense of how this would > play out in the real world. I continue to suspect that we're making > a mountain out of, if not a molehill, at least a hillock. I think most > sane people would have parenthesized their queries to start with rather > than go look up whether IS DISTINCT FROM binds tighter than <= ... This thread seems to have died off without any clear resolution. I'd hoped somebody would try the patch on some nontrivial application to see if it broke anything or caused any warnings, but it doesn't seem like that is happening. Do we have consensus on doing this? Should we have the warning on by default, or off? 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] Precedence of standard comparison operators
On 2015-02-26 20:13:34 +, Simon Riggs wrote: > On 26 February 2015 at 15:56, Tom Lane wrote: > > >> I think the way to do this is to have a pluggable parser, so users can > >> choose 1) old parser, 2) new, better parser, 3) any other parser they > >> fancy that they maintain to ensure application compatibility. We've > >> got a pluggable executor and optimizer, so why not a parser too? > >> Implementing that in the same way we have done for executor and > >> optimizer is a 1 day patch, so easily achievable for 9.5. > > > > I don't find that particularly attractive either. It seems quite unlikely > > to me that anyone would actually use such a hook; replacing the whole > > parser would be essentially unmaintainable. Nobody uses the hooks you > > mention to replace the whole planner or whole executor --- there are > > wrappers out there, which is a different thing altogether. But you could > > not undo the issue at hand here without at the very least a whole new > > copy of gram.y, which would need to be updated constantly if you wanted > > it to keep working across more than one release. I can see a point in having the ability to plug in a parser - I just don't think it has much to do with this topic. It'd be a nightmare to maintain two versions of our parser; I don't think anybody wants to patch more than one. > Whole planner replacement is used for extensions by CitusData and NTT, > and will definitely be used in the future for other apps. s/planner/parser/? Because replacing the planner can already be done as a whole without much problems. > Whole executor replacement is also used by one extension to produce > DDL triggers. Hm? > In any case, whole planner replacement would be very desirable for > people trying to minimize code churn in their applications. As soon as > it exists, I will use to make some MySQL-only apps work seamlessly > against PostgreSQL, such as WordPress. It doesn't need to be 100% > perfect MySQL, it just needs to be enough to make the app work. > Maintaining a code base that can work against multiple databases is > hard. Writing it against one main database and maintaining a parser > layer would be much easier. Assuming you meant parser: Maybe. I have my doubt somebody will actually invest the significant amount of time to develop something like that. But I don't have a problem providing the capability; there seems little point in allowing to replace the optimizer but not the planner. I just don't see that it has much to do with this discussion. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] Precedence of standard comparison operators
On 2/26/15 4:09 PM, Tom Lane wrote: Andres Freund writes: On February 26, 2015 10:29:18 PM CET, Peter Eisentraut wrote: My suggestion was to treat this like the standard_conforming_string change. That is, warn for many years before changing. I don't think scs is a good example to follow. Yeah. For one thing, there wouldn't be any way to suppress the warning other than to parenthesize your code, which I would find problematic because it would penalize standard-conforming queries. I'd prefer an arrangement whereby once you fix your code to be standard-conforming, you're done. A possible point of compromise would be to leave the warning turned on by default, at least until we get a better sense of how this would play out in the real world. I continue to suspect that we're making a mountain out of, if not a molehill, at least a hillock. I think most sane people would have parenthesized their queries to start with rather than go look up whether IS DISTINCT FROM binds tighter than <= ... Question of sanity aside, I can tell you that many business queries are written with little more sophistication than monkeys with typewriters, where you keep the monkeys fed with bananas and paper until your query results (not the SQL) looks like what someone expects it to look like. Then the output of that version is held as sacrosanct, and any future SQL changes are wrong unless they produce the expected result changes. In my experience this happens because some poor business person just needs to get their job done and either isn't allowed to bother the data people or the data people are just too busy, so they're stuck doing it themselves. From what I've seen, SQL written by developers is often a bit better than this... but not a lot. And part of that salvation is because application queries tend to be a lot less complex than reporting/BI ones. I don't have a great feel for how much of an impact this specific change would have on that... the examples so far have all been pretty esoteric. I suspect most people writing such "wonderful" SQL don't know about IS DISTINCT FROM nor would they try doing things like bool_a > bool_b >= bool_c. So there may actually be very little code to fix, but I think we at least need a way for users to verify that. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- 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] Precedence of standard comparison operators
Andres Freund writes: > On February 26, 2015 10:29:18 PM CET, Peter Eisentraut > wrote: >> My suggestion was to treat this like the standard_conforming_string >> change. That is, warn for many years before changing. > I don't think scs is a good example to follow. Yeah. For one thing, there wouldn't be any way to suppress the warning other than to parenthesize your code, which I would find problematic because it would penalize standard-conforming queries. I'd prefer an arrangement whereby once you fix your code to be standard-conforming, you're done. A possible point of compromise would be to leave the warning turned on by default, at least until we get a better sense of how this would play out in the real world. I continue to suspect that we're making a mountain out of, if not a molehill, at least a hillock. I think most sane people would have parenthesized their queries to start with rather than go look up whether IS DISTINCT FROM binds tighter than <= ... 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] Precedence of standard comparison operators
On February 26, 2015 10:29:18 PM CET, Peter Eisentraut wrote: >On 2/25/15 5:15 PM, Simon Riggs wrote: >> Having a warn_if_screwed parameter that we disable by default won't >> help much because if you are affected you can't change that >situation. >> There are too many applications to test all of them and not all >> applications can be edited, even if they were tested. > >My suggestion was to treat this like the standard_conforming_string >change. That is, warn for many years before changing. I don't think scs is a good example to follow. For one it didn't work well. For another the impact of scs was imo bigger and had security implications. Also people didn't really switch because of it until the default changed. At the very least there should be an option to error out, not warn. --- Please excuse brevity and formatting - I am writing this on my mobile phone. -- 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] Precedence of standard comparison operators
On 2/25/15 5:15 PM, Simon Riggs wrote: > Having a warn_if_screwed parameter that we disable by default won't > help much because if you are affected you can't change that situation. > There are too many applications to test all of them and not all > applications can be edited, even if they were tested. My suggestion was to treat this like the standard_conforming_string change. That is, warn for many years before changing. -- 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] Precedence of standard comparison operators
Simon Riggs writes: > On 26 February 2015 at 15:56, Tom Lane wrote: >> I don't find that particularly attractive either. It seems quite unlikely >> to me that anyone would actually use such a hook; replacing the whole >> parser would be essentially unmaintainable. Nobody uses the hooks you >> mention to replace the whole planner or whole executor --- there are >> wrappers out there, which is a different thing altogether. But you could >> not undo the issue at hand here without at the very least a whole new >> copy of gram.y, which would need to be updated constantly if you wanted >> it to keep working across more than one release. > That's not what I see. > Whole planner replacement is used for extensions by CitusData and NTT, > and will definitely be used in the future for other apps. > Whole executor replacement is also used by one extension to produce > DDL triggers. That sounds completely silly from here. You'd be better off maintaining your code as a set of patches on the core code and shipping your own binaries. It would be substantially less work to maintain, I'd think, because otherwise you have to track every single patch made to what are very large swaths of code. The costs of merge resolution (when your changes specifically touch something also updated by the community) would be about the same, but the gruntwork aspect would be considerably better. And you'd have to be certifiably insane to ship such things as extensions not tied to specific core binaries, anyway, so I'm not seeing an upside in doing it like that. Having said that, I won't stand in the way of somebody else proposing parser hooks. I don't intend to do it myself though. 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] Precedence of standard comparison operators
On 26 February 2015 at 15:56, Tom Lane wrote: >> I think the way to do this is to have a pluggable parser, so users can >> choose 1) old parser, 2) new, better parser, 3) any other parser they >> fancy that they maintain to ensure application compatibility. We've >> got a pluggable executor and optimizer, so why not a parser too? >> Implementing that in the same way we have done for executor and >> optimizer is a 1 day patch, so easily achievable for 9.5. > > I don't find that particularly attractive either. It seems quite unlikely > to me that anyone would actually use such a hook; replacing the whole > parser would be essentially unmaintainable. Nobody uses the hooks you > mention to replace the whole planner or whole executor --- there are > wrappers out there, which is a different thing altogether. But you could > not undo the issue at hand here without at the very least a whole new > copy of gram.y, which would need to be updated constantly if you wanted > it to keep working across more than one release. That's not what I see. Whole planner replacement is used for extensions by CitusData and NTT, and will definitely be used in the future for other apps. Whole executor replacement is also used by one extension to produce DDL triggers. In any case, whole planner replacement would be very desirable for people trying to minimize code churn in their applications. As soon as it exists, I will use to make some MySQL-only apps work seamlessly against PostgreSQL, such as WordPress. It doesn't need to be 100% perfect MySQL, it just needs to be enough to make the app work. Maintaining a code base that can work against multiple databases is hard. Writing it against one main database and maintaining a parser layer would be much easier. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, RemoteDBA, 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] Precedence of standard comparison operators
Andrew Dunstan writes: > On 02/26/2015 10:56 AM, Tom Lane wrote: >> I find this argument to be unhelpful, because it could be made in exactly >> the same words against any non-backwards-compatible change whatsoever. >> Nonetheless, we do make non-backwards-compatible changes all the time. > That's true, we do. But finding out where apps are going to break is not > going to be easy. Reviewing a million lines of code to examine where > changes in operator precendence might affect you could be an enormous > undertaking for many users. I understand the need, but the whole > prospect makes me very, very nervous, TBH. Well, again, it's not apparent to me why such arguments can't be used to prevent us from ever again making any user-visible semantics change. In this particular case, given I've gone to the trouble of creating a warning mode, I think it would be easier to find potential problems than it is for many of the compatibility changes we've made in the past. A not-terribly-far-away example is your own recent change in casting timestamp values to JSON; if someone had demanded a way to audit their applications to find places where that might break things, could that patch have been accepted? Indeed, could *any* of the backwards compatibility breaks called out in the 9.4 notes have passed such a test? I'm not honestly sure why we're holding this particular change to such a high standard. (Also, I think that most cases in practice are going to end up as parse errors, not silently different query results, though I admit I haven't got much evidence one way or the other on that. It'd be useful perhaps if someone tried some large existing application against the patch to see what happens. How many parse failures come up, and how many warnings?) 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] Precedence of standard comparison operators
On 02/26/2015 10:56 AM, Tom Lane wrote: Simon Riggs writes: On 20 February 2015 at 20:44, Tom Lane wrote: Well, assuming that we're satisfied with just having a way to warn when the behavior changed (and not, in particular, a switch that can select old or new behavior) I'm in favour of your proposed improvements, but I'm having a problem thinking about random application breakage that would result. Having a warn_if_screwed parameter that we disable by default won't help much because if you are affected you can't change that situation. There are too many applications to test all of them and not all applications can be edited, even if they were tested. I find this argument to be unhelpful, because it could be made in exactly the same words against any non-backwards-compatible change whatsoever. Nonetheless, we do make non-backwards-compatible changes all the time. That's true, we do. But finding out where apps are going to break is not going to be easy. Reviewing a million lines of code to examine where changes in operator precendence might affect you could be an enormous undertaking for many users. I understand the need, but the whole prospect makes me very, very nervous, TBH. cheers andrew -- 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] Precedence of standard comparison operators
Simon Riggs writes: > On 20 February 2015 at 20:44, Tom Lane wrote: >> Well, assuming that we're satisfied with just having a way to warn >> when the behavior changed (and not, in particular, a switch that can >> select old or new behavior) > I'm in favour of your proposed improvements, but I'm having a problem > thinking about random application breakage that would result. > Having a warn_if_screwed parameter that we disable by default won't > help much because if you are affected you can't change that situation. > There are too many applications to test all of them and not all > applications can be edited, even if they were tested. I find this argument to be unhelpful, because it could be made in exactly the same words against any non-backwards-compatible change whatsoever. Nonetheless, we do make non-backwards-compatible changes all the time. > I think the way to do this is to have a pluggable parser, so users can > choose 1) old parser, 2) new, better parser, 3) any other parser they > fancy that they maintain to ensure application compatibility. We've > got a pluggable executor and optimizer, so why not a parser too? > Implementing that in the same way we have done for executor and > optimizer is a 1 day patch, so easily achievable for 9.5. I don't find that particularly attractive either. It seems quite unlikely to me that anyone would actually use such a hook; replacing the whole parser would be essentially unmaintainable. Nobody uses the hooks you mention to replace the whole planner or whole executor --- there are wrappers out there, which is a different thing altogether. But you could not undo the issue at hand here without at the very least a whole new copy of gram.y, which would need to be updated constantly if you wanted it to keep working across more than one release. 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] Precedence of standard comparison operators
On 20 February 2015 at 20:44, Tom Lane wrote: > Well, assuming that we're satisfied with just having a way to warn > when the behavior changed (and not, in particular, a switch that can > select old or new behavior) I'm in favour of your proposed improvements, but I'm having a problem thinking about random application breakage that would result. Having a warn_if_screwed parameter that we disable by default won't help much because if you are affected you can't change that situation. There are too many applications to test all of them and not all applications can be edited, even if they were tested. I think the way to do this is to have a pluggable parser, so users can choose 1) old parser, 2) new, better parser, 3) any other parser they fancy that they maintain to ensure application compatibility. We've got a pluggable executor and optimizer, so why not a parser too? Implementing that in the same way we have done for executor and optimizer is a 1 day patch, so easily achievable for 9.5. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, RemoteDBA, 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] Precedence of standard comparison operators
Here's a completed patch for this. This includes fixing the NOT LIKE problem as discussed in the other thread. I've done more-or-less-exhaustive testing on this to verify that it produces warnings whenever necessary. It generates a few false-positive warnings in corner cases that seem too complicated to model more precisely. An example is that the production for "= ANY (sub-select)" clauses looks like a_expr subquery_Op sub_type select_with_parens%prec Op but in point of fact its precedence against operators to its left is not necessarily Op, but whatever actual operator appears --- for example "= ANY" has the precedence of "=". This is because of the same point noted in the other thread that Bison really implements that by comparison to the lookahead token's precedence, not the rule's declared precedence. The patch treats this as having precedence Op, which is the highest possibility, so it will sometimes warn unnecessarily. regards, tom lane diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 9261e7f..cd89819 100644 *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** dynamic_library_path = 'C:\tools\postgre *** 6792,6797 --- 6792,6820 + + operator_precedence_warning (boolean) + +operator_precedence_warning configuration parameter + + + + + When on, the parser will emit a warning for any construct that might + have changed meanings since PostgreSQL 9.4 as a result + of changes in operator precedence. This is useful for auditing + applications to see if precedence changes have broken anything; but it + is not meant to be kept turned on in production, since it will warn + about some perfectly valid, standard-compliant SQL code. + The default is off. + + + + See for more information. + + + + quote_all_identifiers (boolean) diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index 4b81b08..e492684 100644 *** a/doc/src/sgml/syntax.sgml --- b/doc/src/sgml/syntax.sgml *** CAST ( 'stringPostgreSQL. Most operators have the same precedence and are left-associative. The precedence and associativity of the operators is hard-wired ! into the parser. This can lead to non-intuitive behavior; for ! example the Boolean operators < and ! > have a different precedence than the Boolean ! operators <= and >=. Also, you will sometimes need to add parentheses when using combinations of binary and unary operators. For instance: --- 984,994 associativity of the operators in PostgreSQL. Most operators have the same precedence and are left-associative. The precedence and associativity of the operators is hard-wired ! into the parser. ! ! ! ! You will sometimes need to add parentheses when using combinations of binary and unary operators. For instance: *** SELECT (5 !) - 6; *** 1008,1014 ! Operator Precedence (decreasing) --- 1009,1015 ! Operator Precedence (highest to lowest) *** SELECT (5 !) - 6; *** 1063,1125 !IS ! !IS TRUE, IS FALSE, IS NULL, etc ! ! ! !ISNULL ! !test for null ! ! ! !NOTNULL ! !test for not null ! ! ! !(any other) left all other native and user-defined operators -IN - -set membership - - - -BETWEEN - -range containment - - - OVERLAPS time interval overlap !LIKE ILIKE SIMILAR !string pattern matching !< > !less than, greater than != !right !equality, assignment --- 1064,1098 !(any other operator) left all other native and user-defined operators OVERLAPS time interval overlap !BETWEEN IN LIKE ILIKE SIMILAR !range containment, set membership, string matching !< > = <= >= <> ! !comparison operators !IS ISNULL NOTNULL ! !IS TRUE, IS FALSE, IS !NULL, IS DISTINCT FROM, etc *** SELECT (5 !) - 6; *** 1159,1167 SELECT 3 OPERATOR(pg_catalog.+) 4; the OPERATOR construct is taken to have the default precedence ! shown in for any other operator. T
Re: [HACKERS] Precedence of standard comparison operators
Attached is an improved patch that includes optional warnings for constructs that changed parsing. It's not quite 100% but I think it's about 90% correct; the difference in size between this and the previous patch should be a pretty fair indication of what it's going to cost us to have a warning capability. What's missing from this version is that it can't tell the difference between LIKE/ILIKE/SIMILAR TO and the underlying operators, that is, it sees "a LIKE b" as "a ~~ b" because that's what the grammar emits. However, those inputs have different operator-precedence behavior. Likewise, we can't tell the difference between xmlexpr IS NOT DOCUMENT NOT (xmlexpr IS DOCUMENT) because the grammar converts the former into the latter --- and again, those two things have different precedence behavior. It wouldn't take very much additional code to fix these things by changing what the grammar emits; but I'm running out of energy for today. In any case, I thought I should put this up and see if this general approach is going to satisfy people's concerns about making such a change. regards, tom lane diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6bcb106..22a2d32 100644 *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** dynamic_library_path = 'C:\tools\postgre *** 6752,6757 --- 6752,6780 + + operator_precedence_warning (boolean) + +operator_precedence_warning configuration parameter + + + + + When on, the parser will emit a warning for any construct that might + have changed meanings since PostgreSQL 9.4 as a result + of changes in operator precedence. This is useful for auditing + applications to see if precedence changes have broken anything; but it + is not meant to be left turned on in production, since it will warn + about some perfectly valid, standard-compliant SQL code. + The default is off. + + + + See for more information. + + + + quote_all_identifiers (boolean) diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index 4b81b08..e7484c4 100644 *** a/doc/src/sgml/syntax.sgml --- b/doc/src/sgml/syntax.sgml *** CAST ( 'stringPostgreSQL. Most operators have the same precedence and are left-associative. The precedence and associativity of the operators is hard-wired ! into the parser. This can lead to non-intuitive behavior; for ! example the Boolean operators < and ! > have a different precedence than the Boolean ! operators <= and >=. Also, you will sometimes need to add parentheses when using combinations of binary and unary operators. For instance: --- 984,994 associativity of the operators in PostgreSQL. Most operators have the same precedence and are left-associative. The precedence and associativity of the operators is hard-wired ! into the parser. ! ! ! ! You will sometimes need to add parentheses when using combinations of binary and unary operators. For instance: *** SELECT (5 !) - 6; *** 1008,1014 ! Operator Precedence (decreasing) --- 1009,1015 ! Operator Precedence (highest to lowest) *** SELECT (5 !) - 6; *** 1063,1087 !IS ! !IS TRUE, IS FALSE, IS NULL, etc ! ! ! !ISNULL ! !test for null ! ! ! !NOTNULL ! !test for not null ! ! ! !(any other) left all other native and user-defined operators --- 1064,1070 !(any other operator) left all other native and user-defined operators *** SELECT (5 !) - 6; *** ,1125 !< > !less than, greater than != !right !equality, assignment --- 1094,1110 !< > = <= >= <> ! !comparison operators !IS ISNULL NOTNULL ! !IS TRUE, IS FALSE, IS !NULL, IS DISTINCT FROM, etc *** SELECT (5 !) - 6; *** 1159,1167 SELECT 3 OPERATOR(pg_catalog.+) 4; the OPERATOR construct is taken to have the default precedence ! shown in for any other operator. This is true no matter which specific operator appears inside OPERATOR(). --- 1144,1172 SELECT 3 OPERATOR(pg_catalog.+) 4; the OPERATOR construct is taken to have the default precedence ! shown in for ! any other operator. This is true no
Re: [HACKERS] Precedence of standard comparison operators
I wrote: > Attached is a draft patch to bring the precedence of comparison operators > and IS tests into line with the SQL standard. I have not yet looked into > producing warnings for changes in parsing decisions ... I've made some progress on getting parse_expr.c to produce warnings by after-the-fact analysis of the raw parse tree, and I think it can be made to work. However, I've run into two stumbling blocks that greatly limit the quality of the warnings, and will need to be fixed as separate patches: 1. BooleanTest and NullTest node types don't carry location pointers, so there's no way to give an error cursor position referencing them. Simple enough addition. I think we left these out because there were no post-grammar error reports involving them, but now we need some. 2. gram.y expands BETWEEN operations into complex AND/OR nests on sight, losing the information that there was a BETWEEN there, which is important if we're to detect possible precedence changes involving BETWEEN. What we should do about #2 is preserve BETWEEN as a distinct construct in the output of raw parsing. This is a good thing anyway, because in the long run we are going to want to fix BETWEEN's semantic issues (such as multiple evaluation of possibly-volatile input expressions) and fixing what the grammar does with it is an essential first step. Barring objection I'll go do both of those things as separate patches, and then come back to the precedence warnings. 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] Precedence of standard comparison operators
Attached is a draft patch to bring the precedence of comparison operators
and IS tests into line with the SQL standard. I have not yet looked into
producing warnings for changes in parsing decisions; but I was gratified
to discover that this patch results in none, nada, zero changes in any
of our regression tests. So that's at least some evidence that it may
not be a huge problem in practice. Pending writing some code for warnings,
I thought I'd throw this up in case anyone wants to try it on applications
they have handy.
Credit where credit is due dept: this is mostly the work of Serge Rielau
of Salesforce.
regards, tom lane
diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml
index 4b81b08..c88e7d9 100644
*** a/doc/src/sgml/syntax.sgml
--- b/doc/src/sgml/syntax.sgml
*** CAST ( 'stringPostgreSQL.
Most operators have the same precedence and are left-associative.
The precedence and associativity of the operators is hard-wired
! into the parser. This can lead to non-intuitive behavior; for
! example the Boolean operators < and
! > have a different precedence than the Boolean
! operators <= and >=. Also, you will
sometimes need to add parentheses when using combinations of
binary and unary operators. For instance:
--- 984,994
associativity of the operators in PostgreSQL.
Most operators have the same precedence and are left-associative.
The precedence and associativity of the operators is hard-wired
! into the parser.
!
!
!
! You will
sometimes need to add parentheses when using combinations of
binary and unary operators. For instance:
*** SELECT (5 !) - 6;
*** 1063,1087
!IS
!
!IS TRUE, IS FALSE, IS NULL, etc
!
!
!
!ISNULL
!
!test for null
!
!
!
!NOTNULL
!
!test for not null
!
!
!
!(any other)
left
all other native and user-defined operators
--- 1064,1070
!(any other operator)
left
all other native and user-defined operators
*** SELECT (5 !) - 6;
*** ,1125
!< >
!less than, greater than
!=
!right
!equality, assignment
--- 1094,1109
!< > = <= >= <>
!
!comparison operators
!IS ISNULL NOTNULL
!
!IS TRUE, IS FALSE, IS NULL, etc
*** SELECT (5 !) - 6;
*** 1159,1165
SELECT 3 OPERATOR(pg_catalog.+) 4;
the OPERATOR construct is taken to have the default precedence
! shown in for any other operator. This is true no matter
which specific operator appears inside OPERATOR().
--- 1143,1150
SELECT 3 OPERATOR(pg_catalog.+) 4;
the OPERATOR construct is taken to have the default precedence
! shown in for
! any other operator. This is true no matter
which specific operator appears inside OPERATOR().
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 36dac29..f98158c 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*** static Node *makeRecursiveViewSelect(cha
*** 532,537
--- 532,538
%token IDENT FCONST SCONST BCONST XCONST Op
%token ICONST PARAM
%token TYPECAST DOT_DOT COLON_EQUALS
+ %token LESS_EQUALS GREATER_EQUALS NOT_EQUALS
/*
* If you want to make any keyword changes, update the keyword table in
*** static Node *makeRecursiveViewSelect(cha
*** 645,652
%left OR
%left AND
%right NOT
! %right '='
! %nonassoc '<' '>'
%nonassoc LIKE ILIKE SIMILAR
%nonassoc ESCAPE
%nonassoc OVERLAPS
--- 646,653
%left OR
%left AND
%right NOT
! %nonassoc IS ISNULL NOTNULL /* IS sets precedence for IS NULL, etc */
! %nonassoc '<' '>' '=' LESS_EQUALS GREATER_EQUALS NOT_EQUALS
%nonassoc LIKE ILIKE SIMILAR
%nonassoc ESCAPE
%nonassoc OVERLAPS
*** static Node *makeRecursiveViewSelect(cha
*** 676,684
%nonassoc UNBOUNDED /* ideally should have same precedence as IDENT */
%nonassoc IDENT NULL_P PARTITION RANGE ROWS PRECEDING FOLLOWING
%left Op OPERATOR /* multi-character ops and user-defined operators */
- %nonassoc NOTNULL
- %nonassoc ISNULL
- %nonassoc IS/* sets precedence for IS NULL, etc */
%left '+' '-'
%left '*' '/' '%'
%left '^'
--- 677,682
*** a_expr: c_expr { $$ = $1; }
*** 11202,11207
--- 11200,11211
{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, ">", $1, $3, @2); }
| a_expr '=' a_expr
{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "=", $1, $3, @2); }
+ | a_expr LESS_EQUALS a_expr
+ {
Re: [HACKERS] Precedence of standard comparison operators
Peter Eisentraut writes: > We could check if there is a >= or <= as a child of another general > operator. That is already quite unlikely to begin with (except for the > obvious common case I am forgetting right now). We could even do this > in an external module with a hook. Or to be more precise, check whether > the >= or <= was in parentheses, which we could record in the parser. > Neither might be absolutely accurate, but it would at least give users a > list of things to check. > The above would imply that we add these checks before changing the > precedence. Creating a check under the new precendence would be much > harder. Hm. Well, assuming that we're satisfied with just having a way to warn when the behavior changed (and not, in particular, a switch that can select old or new behavior), I think it might not be that hard. The point is that we're going to reduce the precedence of <= and friends, which means that some other operator that might formerly have bound less tightly might now bind more tightly and thereby be underneath the <= instead of above it. So it seems like we could adapt your idea: in transformAExprOp, if the opname is <= etc, then check to see if the righthand argument is another A_Expr with a multicharacter Op name. If so warn. As you said, we'd need to mark parenthesized subexpressions but that might not be horridly painful. 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] Precedence of standard comparison operators
On 2/20/15 2:41 PM, Tom Lane wrote: > I don't believe there is any practical way for us to generate useful > warnings here; as I said to Kevin, I don't think that Bison exposes > sufficient information to detect when a parsing decision was made > differently than before because of precedence. We could check if there is a >= or <= as a child of another general operator. That is already quite unlikely to begin with (except for the obvious common case I am forgetting right now). We could even do this in an external module with a hook. Or to be more precise, check whether the >= or <= was in parentheses, which we could record in the parser. Neither might be absolutely accurate, but it would at least give users a list of things to check. The above would imply that we add these checks before changing the precedence. Creating a check under the new precendence would be much harder. -- 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] Precedence of standard comparison operators
Peter Eisentraut writes: > I think we should try to do it, but we need a way for users to see what > is going on. If we just put into the release notes, "the precedences of >> = and <= have been changed, but we don't expect this to cause many > problems", there might be wide-spread panic. > One way would be to have a knob that warns/logs/debugs when it sees an > <= or >= call in a place that would change meaning. Perhaps in > transformAExprOp(). This might be an expensive check, but it wouldn't > have to be on all the time. We could also add a flag to the A_Expr node > that remember whether the expression was parenthesized, so that users > could update their code with parentheses to shut the warning up. > I think this would be a standard_conforming_strings-like transition. We had this discussion back in 2007 :-(. I don't believe there is any practical way for us to generate useful warnings here; as I said to Kevin, I don't think that Bison exposes sufficient information to detect when a parsing decision was made differently than before because of precedence. If there's going to be an insistence on that then I suspect we'll spend another 8 years not conforming to spec. 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] Precedence of standard comparison operators
On 2/19/15 10:48 AM, Tom Lane wrote: > I've not really experimented with this at all; it would be useful for > example to see how many regression tests break as a gauge for how > troublesome such changes would be. I thought I'd ask whether there's > any chance at all of such a change getting accepted before doing any > serious work on it. I think we should try to do it, but we need a way for users to see what is going on. If we just put into the release notes, "the precedences of >= and <= have been changed, but we don't expect this to cause many problems", there might be wide-spread panic. One way would be to have a knob that warns/logs/debugs when it sees an <= or >= call in a place that would change meaning. Perhaps in transformAExprOp(). This might be an expensive check, but it wouldn't have to be on all the time. We could also add a flag to the A_Expr node that remember whether the expression was parenthesized, so that users could update their code with parentheses to shut the warning up. I think this would be a standard_conforming_strings-like transition. -- 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] Precedence of standard comparison operators
Tom Lane wrote: > One of the reasons I want to make these operators %nonassoc is > so you get an error on cases like these --- if you actually meant > this, you'll be forced to parenthesize one way or the other. I could live with that versus a configurable warning. It's simpler and makes it less likely that someone will accidentally get incorrect results without realizing it. If we confirm that the standard specifies a left-to-right evaluation (which I seem to recall, but wouldn't trust that memory without confirmation), we could consider loosening it up five or ten years down the road, once everyone has code that works with this stricter implementation. -- 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] Precedence of standard comparison operators
Kevin Grittner wrote: > Tom Lane wrote: >> Kevin Grittner writes: >>> Tom Lane wrote: the precedence of <= >= and <> is neither sane nor standards compliant. >>> >>> I wonder whether it would be feasible to have an option to generate >>> warnings (or maybe just LOG level messages?) for queries where the >>> results could differ. >> >> My guess (admittedly not yet based on much) is that warnings won't be too >> necessary. If a construction is parsed differently than before, you'll >> get no-such-operator gripes. > > I have a memory of running into this in real-world production code > and that it involved booleans. I'll see whether I posted something > to the community lists about it [...] Here's what I posted when I ran into it in real-world code, although I posted simplified test cases rather than the (probably very complex) production code: http://www.postgresql.org/message-id/200712171958.lbhjwobb037...@wwwmaster.postgresql.org -- 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] Precedence of standard comparison operators
Kevin Grittner writes: > I have a memory of running into this in real-world production code > and that it involved booleans. I'll see whether I posted something > to the community lists about it, but it didn't take long to produce > an (admittedly artificial) case where incorrect results are > silently returned: > test=# select 'f'::boolean = 'f'::boolean >= 'f'::boolean; > ?column? > -- > f > (1 row) > test=# select 'f'::boolean >= 'f'::boolean >= 'f'::boolean; > ?column? > -- > t > (1 row) One of the reasons I want to make these operators %nonassoc is so you get an error on cases like these --- if you actually meant this, you'll be forced to parenthesize one way or the other. 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] Precedence of standard comparison operators
Tom Lane wrote: > Kevin Grittner writes: >> Tom Lane wrote: >>> the precedence of <= >= and <> is neither sane nor standards compliant. >> I wonder whether it would be feasible to have an option to generate >> warnings (or maybe just LOG level messages?) for queries where the >> results could differ. > > My guess (admittedly not yet based on much) is that warnings won't be too > necessary. If a construction is parsed differently than before, you'll > get no-such-operator gripes. I have a memory of running into this in real-world production code and that it involved booleans. I'll see whether I posted something to the community lists about it, but it didn't take long to produce an (admittedly artificial) case where incorrect results are silently returned: test=# select 'f'::boolean = 'f'::boolean >= 'f'::boolean; ?column? -- f (1 row) test=# select 'f'::boolean >= 'f'::boolean >= 'f'::boolean; ?column? -- t (1 row) -- 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] Precedence of standard comparison operators
Kevin Grittner writes: > Tom Lane wrote: >> the precedence of <= >= and <> is neither sane nor standards compliant. > I wonder whether it would be feasible to have an option to generate > warnings (or maybe just LOG level messages?) for queries where the > results could differ. My guess (admittedly not yet based on much) is that warnings won't be too necessary. If a construction is parsed differently than before, you'll get no-such-operator gripes. The case of interest is something like a <= b %% c which was formerly (a <= b) %% c and would become a <= (b %% c) Now, if it worked before, %% must expect a boolean left input; but the odds are pretty good that b is not boolean. This argument does get a lot weaker when you consider operators that take nearly anything, such as ||; for instance if a b c are all text then both parsings of a <= b || c are type-wise acceptable. But that's something that I hope most people would've parenthesized to begin with, because (a <= b) || c is not exactly the intuitive expectation for what you'll get. Anyway, to answer your question, I think that Bison knows somewhere inside when it's making a precedence-driven choice like this, but I don't believe it's exposed in any way that we could get at easily. Perhaps there would be a way to produce a warning if we hand-hacked the C-code bison output, but we're not gonna do that. 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] Precedence of standard comparison operators
Tom Lane wrote: > the precedence of <= >= and <> is neither sane nor standards compliant. +1 That was a bit of a pain when I migrated a lot of code from Sybase ASE to PostgreSQL; I think we should conform to the standard on this, even if it breaks backward compatibility. (Of course, the release notes must warn about this in a conspicuous way.) > We have that right for = < > but not for the other three standard-mandated > comparison operators. I think we should change the grammar so that all > six act like < > do now, that is, they should have %nonassoc precedence > just above NOT. +1 > Another thought, looking at this closely, is that we have the precedence > of IS tests (IS NOT NULL etc) wrong as well: they should bind less tightly > than user-defined ops, not more so. I'm less excited about changing that, > but there's certainly room to argue that it's wrong per spec. It's > definitely weird that the IS tests bind more tightly than multicharacter > Ops but less tightly than + - * /. Again, I think it is worth it to conform to the standard. > I've not really experimented with this at all; it would be useful for > example to see how many regression tests break as a gauge for how > troublesome such changes would be. I thought I'd ask whether there's > any chance at all of such a change getting accepted before doing any > serious work on it. You will have my vote. I wonder whether it would be feasible to have an option to generate warnings (or maybe just LOG level messages?) for queries where the results could differ. (I seem to remember some queries that, when written to standard and run on PostgreSQL, silently returned results incompatible with the standard. We changed the code to generate SQL to emit extra layers of parentheses to get the same behavior on both databases, but we had to notice the problem first.) -- 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
[HACKERS] Precedence of standard comparison operators
My Salesforce colleagues have been bugging me about this topic, and since I see in a nearby thread that we may be about to break backwards compatibility on "=>", maybe it's time to do something about this too. To wit, that the precedence of <= >= and <> is neither sane nor standards compliant. Up to now, Postgres has had special precedence rules for = < and >, but their multi-character brethren just get treated as general Op tokens. This has assorted surprising consequences; for example you can do this: regression=# select * from testjsonb where j->>'space' < j->>'node'; but not this: regression=# select * from testjsonb where j->>'space' <= j->>'node'; ERROR: operator does not exist: text <= jsonb LINE 1: select * from testjsonb where j->>'space' <= j->>'node'; ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. Of course the latter happens because ->> and <= have identical precedence so the construct is parsed as ((j ->> 'space') <= j) ->> 'node' whereas < has lower precedence than user-defined operators so the first case parses in the expected fashion. I claim that this behavior is contrary to spec as well as being unintuitive. Following the grammar productions in SQL99: ::= WHERE ::= ::= | OR ::= | AND ::= [ NOT ] ::= [ IS [ NOT ] ] ::= TRUE | FALSE | UNKNOWN ::= | | ::= ::= | | | | | | | | | | | | ::= ::= | | | | | ::= | ::= | ::= | So both the examples I gave should be understood as value expressions related by s. This line of reasoning says that any non-boolean operator should bind tighter than the six standard comparison operators, because it will necessarily be part of a component of a boolean expression. We have that right for = < > but not for the other three standard-mandated comparison operators. I think we should change the grammar so that all six act like < > do now, that is, they should have %nonassoc precedence just above NOT. Another thought, looking at this closely, is that we have the precedence of IS tests (IS NOT NULL etc) wrong as well: they should bind less tightly than user-defined ops, not more so. I'm less excited about changing that, but there's certainly room to argue that it's wrong per spec. It's definitely weird that the IS tests bind more tightly than multicharacter Ops but less tightly than + - * /. I've not really experimented with this at all; it would be useful for example to see how many regression tests break as a gauge for how troublesome such changes would be. I thought I'd ask whether there's any chance at all of such a change getting accepted before doing any serious work on it. Thoughts? 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
