new "row-level lock" error messages

Started by Peter Eisentrautover 12 years ago7 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

In 9af4159f in combination with cb9b66d3 a bunch of error messages were
changed from something like

"SELECT FOR UPDATE/SHARE is not allowed with UNION/INTERSECT/EXCEPT"

to

"row-level locks are not allowed with UNION/INTERSECT/EXCEPT"

because the intermediate state of

"SELECT FOR UPDATE/SHARE/KEY UPDATE/KEY SHARE is not allowed with
UNION/INTERSECT/EXCEPT"

was presumably considered too bulky.

I think that went too far in some cases. For example, the new message

"row-level locks must specify unqualified relation names"

has little to do with its original meaning.

In general, I find these new wordings to be a loss of clarity. There is
no indication on the SELECT man page or in the documentation index what
a "row-level lock" is at all.

I would suggest that these changes be undone, except that the old
"SELECT FOR ..." be replaced by a dynamic string that reverse-parses the
LockingClause to provide the actual clause that was used.

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

#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
Re: new "row-level lock" error messages

Peter Eisentraut wrote:

In general, I find these new wordings to be a loss of clarity. There is
no indication on the SELECT man page or in the documentation index what
a "row-level lock" is at all.

I would suggest that these changes be undone, except that the old
"SELECT FOR ..." be replaced by a dynamic string that reverse-parses the
LockingClause to provide the actual clause that was used.

Hmm, that's an idea. If there are no objections, I'll get this fixed.

--
�lvaro Herrera 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

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
Re: new "row-level lock" error messages

Peter Eisentraut wrote:

I would suggest that these changes be undone, except that the old
"SELECT FOR ..." be replaced by a dynamic string that reverse-parses the
LockingClause to provide the actual clause that was used.

Here's a patch for this.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

lcs.patchtext/x-diff; charset=us-asciiDownload+118-22
#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#3)
Re: new "row-level lock" error messages

Alvaro Herrera wrote:

Peter Eisentraut wrote:

I would suggest that these changes be undone, except that the old
"SELECT FOR ..." be replaced by a dynamic string that reverse-parses the
LockingClause to provide the actual clause that was used.

Here's a patch for this.

Pushed to 9.3 and master. Sample output:

alvherre=# select * from foo, bar for update of foof for share of bar;
ERROR: relation "foof" in FOR UPDATE clause not found in FROM clause

alvherre=# select * from foo, bar for update of foo for share of barf;
ERROR: relation "barf" in FOR SHARE clause not found in FROM clause

Amusingly, the only test in which these error messages appeared, in
contrib/file_fdw, was removed after the two commits that changed the
wording. So there's not a single test which needed to be tweaked for
this change.

--
�lvaro Herrera 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

#5Robert Haas
robertmhaas@gmail.com
In reply to: Alvaro Herrera (#4)
Re: new "row-level lock" error messages

On Tue, Jul 23, 2013 at 2:16 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Alvaro Herrera wrote:

Peter Eisentraut wrote:

I would suggest that these changes be undone, except that the old
"SELECT FOR ..." be replaced by a dynamic string that reverse-parses the
LockingClause to provide the actual clause that was used.

Here's a patch for this.

Pushed to 9.3 and master. Sample output:

alvherre=# select * from foo, bar for update of foof for share of bar;
ERROR: relation "foof" in FOR UPDATE clause not found in FROM clause

alvherre=# select * from foo, bar for update of foo for share of barf;
ERROR: relation "barf" in FOR SHARE clause not found in FROM clause

Amusingly, the only test in which these error messages appeared, in
contrib/file_fdw, was removed after the two commits that changed the
wording. So there's not a single test which needed to be tweaked for
this change.

The fact that there are no tests of this functionality is probably not
a good thing. We should add some. At the moment, the following test
case crashes, and it looks like this commit is responsible:

create table test_update2 (id integer);
DECLARE test_update_cursor CURSOR FOR SELECT id, MIN(id) FROM
test_update2 GROUP By id HAVING MIN(id) < 1 FOR UPDATE;

--
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

#6Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Robert Haas (#5)
Re: new "row-level lock" error messages

Robert Haas escribi�:

The fact that there are no tests of this functionality is probably not
a good thing. We should add some.

No disagreement.

At the moment, the following test
case crashes, and it looks like this commit is responsible:

create table test_update2 (id integer);
DECLARE test_update_cursor CURSOR FOR SELECT id, MIN(id) FROM
test_update2 GROUP By id HAVING MIN(id) < 1 FOR UPDATE;

Grr. Thanks. Will fix.

--
�lvaro Herrera 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

#7Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Robert Haas (#5)
Re: new "row-level lock" error messages

Robert Haas escribi�:

The fact that there are no tests of this functionality is probably not
a good thing. We should add some. At the moment, the following test
case crashes, and it looks like this commit is responsible:

create table test_update2 (id integer);
DECLARE test_update_cursor CURSOR FOR SELECT id, MIN(id) FROM
test_update2 GROUP By id HAVING MIN(id) < 1 FOR UPDATE;

The attached patch fixes it, and I think it makes more sense than the
original coding to start with.

I will commit this tomorrow, adding a few test cases while at it.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

lock-strength.patchtext/x-diff; charset=us-asciiDownload+12-18