operator_precedence_warning vs make installcheck

Started by Jeff Janesalmost 9 years ago4 messages
#1Jeff Janes
jeff.janes@gmail.com
1 attachment(s)

make installcheck fails against a server running with
operator_precedence_warning
= on.

The difference is in update.out, and consists of an error-locating carat
getting moved over by one position. I've attached the regression diff.

I don't know why the setting of this GUC causes the carat to move around,
that seems odd. If it can't be fixed at the source, it should be easy
enough to just override the setting in update.sql.

Cheers,

Jeff

Attachments:

regression.diffsapplication/octet-stream; name=regression.diffsDownload
*** /home/jjanes/pgsql/git/src/test/regress/expected/update.out	Tue Feb 14 12:42:48 2017
--- /home/jjanes/pgsql/git/src/test/regress/results/update.out	Tue Feb 14 15:15:30 2017
***************
*** 148,154 ****
    WHERE update_test.a = v.i;
  ERROR:  source for a multiple-column UPDATE item must be a sub-SELECT or ROW() expression
  LINE 1: UPDATE update_test SET (a,b) = (v.*) FROM (VALUES(21, 101)) ...
!                                         ^
  -- if an alias for the target table is specified, don't allow references
  -- to the original table name
  UPDATE update_test AS t SET b = update_test.b + 10 WHERE t.a = 10;
--- 148,154 ----
    WHERE update_test.a = v.i;
  ERROR:  source for a multiple-column UPDATE item must be a sub-SELECT or ROW() expression
  LINE 1: UPDATE update_test SET (a,b) = (v.*) FROM (VALUES(21, 101)) ...
!                                        ^
  -- if an alias for the target table is specified, don't allow references
  -- to the original table name
  UPDATE update_test AS t SET b = update_test.b + 10 WHERE t.a = 10;

======================================================================

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jeff Janes (#1)
Re: operator_precedence_warning vs make installcheck

Jeff Janes <jeff.janes@gmail.com> writes:

make installcheck fails against a server running with
operator_precedence_warning = on.

The difference is in update.out, and consists of an error-locating carat
getting moved over by one position. I've attached the regression diff.

I don't know why the setting of this GUC causes the carat to move around,
that seems odd.

The reason is that with operator_precedence_warning = on, there's an
explicit raw-parse-tree node for the parenthesis pair, which otherwise
there is not, so that exprLocation reports a different result for the
location of the subexpression.

We could possibly prevent the difference by having exprLocation look
through such nodes. I'm not sure offhand if there are cases where
that would be worse than before. We've definitely made some other
hacks to hide the difference between operator_precedence_warning on
and 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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
Re: operator_precedence_warning vs make installcheck

I wrote:

We could possibly prevent the difference by having exprLocation look
through such nodes. I'm not sure offhand if there are cases where
that would be worse than before. We've definitely made some other
hacks to hide the difference between operator_precedence_warning on
and off.

After some study I concluded the best fix is just to make the AEXPR_PAREN
node have the same reportable location as its child node to begin with.
None of the code dealing with precedence errors was using the location
of the left parenthesis, so there's no good reason to store that.

Pushed a fix along that line.

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

#4Jeff Janes
jeff.janes@gmail.com
In reply to: Tom Lane (#3)
Re: operator_precedence_warning vs make installcheck

On Wed, Feb 15, 2017 at 11:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

We could possibly prevent the difference by having exprLocation look
through such nodes. I'm not sure offhand if there are cases where
that would be worse than before. We've definitely made some other
hacks to hide the difference between operator_precedence_warning on
and off.

After some study I concluded the best fix is just to make the AEXPR_PAREN
node have the same reportable location as its child node to begin with.
None of the code dealing with precedence errors was using the location
of the left parenthesis, so there's no good reason to store that.

Pushed a fix along that line.

regards, tom lane

Thanks.