Re: Suspicious strcmp() in src/backend/parser/parse_expr.c

Andrew Gierth <andrew@tao11.riddles.org.uk>

From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org
Date: 2019-04-10T22:33:35Z
Lists: pgsql-bugs
>>>>> "Rikard" == Rikard Falkeborn <rikard.falkeborn@gmail.com> writes:

 Rikard> In src/backend/parser/parse_expr.c the following snippet of code is found
 Rikard> (lines 3238-3242, rev 765525c8c2c6e55abe):

 Rikard>     if (strcmp(*nodename, "+") == 0 ||
 Rikard>             strcmp(*nodename, "-")) // <-- notice the lack of comparisson
 Rikard> here
 Rikard>             group = 0;
 Rikard>     else
 Rikard>             group = PREC_GROUP_PREFIX_OP;

 Rikard> Should the second part of the || be strcmp(*nodename, "-") ==
 Rikard> 0?

Yes it should.

The effect of this bug is to produce a false operator precedence warning
when those are enabled, like so:

postgres=# set operator_precedence_warning = on;
SET
postgres=# select -random() is null;
WARNING:  operator precedence change: IS is now lower precedence than -

when in fact "-random() is null" always did parse as "(-random()) is null"
making the warning spurious.

-- 
Andrew (irc:RhodiumToad)



Commits

  1. Fix more strcmp() calls using boolean-like comparisons for result checks

  2. Fix backwards test in operator_precedence_warning logic.