[PATCH] Error out if SKIP LOCKED and WITH TIES are both specified
Both bugs #16676[1]/messages/by-id/16676-fd62c3c835880da6@postgresql.org and #17141[2]/messages/by-id/17141-913d78b9675aac8e@postgresql.org illustrate that the combination of SKIP
LOCKED and FETCH FIRST
WITH TIES break expectations when it comes to rows returned to other
sessions accessing the same
row. Since this situation is detectable from the syntax and hard to fix
otherwise, forbid for now,
with the potential to fix in the future.
[1]: /messages/by-id/16676-fd62c3c835880da6@postgresql.org
/messages/by-id/16676-fd62c3c835880da6@postgresql.org
[2]: /messages/by-id/17141-913d78b9675aac8e@postgresql.org
/messages/by-id/17141-913d78b9675aac8e@postgresql.org
Proposed backpatch to 13.
Attachments:
0001-Error-out-if-SKIP-LOCKED-and-WITH-TIES-are-both-spec.patchapplication/octet-stream; name=0001-Error-out-if-SKIP-LOCKED-and-WITH-TIES-are-both-spec.patchDownload
From 9abe9ddf1c784f7da9441d12a268d3acca0fd53e Mon Sep 17 00:00:00 2001
From: David Christensen <david.christensen@crunchydata.com>
Date: Fri, 13 Aug 2021 10:53:51 -0500
Subject: [PATCH] Error out if SKIP LOCKED and WITH TIES are both specified
Both bugs #16676[1] and #17141[2] illustrate that the combination of SKIP LOCKED and FETCH FIRST
WITH TIES break expectations when it comes to rows returned to other sessions accessing the same
row. Since this situation is detectable from the syntax and hard to fix otherwise, forbid for now,
with the potential to fix in the future.
[1] https://www.postgresql.org/message-id/16676-fd62c3c835880da6%40postgresql.org
[2] https://www.postgresql.org/message-id/17141-913d78b9675aac8e%40postgresql.org
Backpatch-through: 13, where WITH TIES was introduced
---
doc/src/sgml/ref/select.sgml | 3 ++-
src/backend/parser/gram.y | 13 +++++++++++++
src/test/regress/expected/limit.out | 5 +++++
src/test/regress/sql/limit.sql | 5 +++++
4 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index fa676b1698..bb0d748403 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -1515,7 +1515,8 @@ FETCH { FIRST | NEXT } [ <replaceable class="parameter">count</replaceable> ] {
The <literal>WITH TIES</literal> option is used to return any additional
rows that tie for the last place in the result set according to
the <literal>ORDER BY</literal> clause; <literal>ORDER BY</literal>
- is mandatory in this case.
+ is mandatory in this case. <literal>WITH TIES</literal> cannot be used
+ in a query with <literal>SKIP LOCKED</literal>.
<literal>ROW</literal> and <literal>ROWS</literal> as well as
<literal>FIRST</literal> and <literal>NEXT</literal> are noise
words that don't influence the effects of these clauses.
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 39a2849eba..99021afe73 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -16806,6 +16806,19 @@ insertSelectOptions(SelectStmt *stmt,
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("WITH TIES cannot be specified without ORDER BY clause")));
+ if (limitClause->limitOption == LIMIT_OPTION_WITH_TIES && stmt->lockingClause)
+ {
+ ListCell *lc;
+
+ foreach (lc, stmt->lockingClause)
+ {
+ LockingClause *lockingClause = lfirst_node(LockingClause, lc);
+ if (lockingClause->waitPolicy == LockWaitSkip)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot use SKIP LOCKED with WITH TIES")));
+ }
+ }
stmt->limitOption = limitClause->limitOption;
}
if (withClause)
diff --git a/src/test/regress/expected/limit.out b/src/test/regress/expected/limit.out
index b75afcc01a..dd8c4b3b2f 100644
--- a/src/test/regress/expected/limit.out
+++ b/src/test/regress/expected/limit.out
@@ -619,6 +619,11 @@ SELECT thousand
0
(2 rows)
+-- SKIP LOCKED and WITH TIES are incompatible
+SELECT thousand
+ FROM onek WHERE thousand < 5
+ ORDER BY thousand FETCH FIRST 1 ROW WITH TIES FOR UPDATE SKIP LOCKED;
+ERROR: cannot use SKIP LOCKED with WITH TIES
-- should fail
SELECT ''::text AS two, unique1, unique2, stringu1
FROM onek WHERE unique1 > 50
diff --git a/src/test/regress/sql/limit.sql b/src/test/regress/sql/limit.sql
index d2d4ef132d..6f0cda9870 100644
--- a/src/test/regress/sql/limit.sql
+++ b/src/test/regress/sql/limit.sql
@@ -173,6 +173,11 @@ SELECT thousand
FROM onek WHERE thousand < 5
ORDER BY thousand FETCH FIRST 2 ROW ONLY;
+-- SKIP LOCKED and WITH TIES are incompatible
+SELECT thousand
+ FROM onek WHERE thousand < 5
+ ORDER BY thousand FETCH FIRST 1 ROW WITH TIES FOR UPDATE SKIP LOCKED;
+
-- should fail
SELECT ''::text AS two, unique1, unique2, stringu1
FROM onek WHERE unique1 > 50
--
2.30.1 (Apple Git-130)
On 2021-Aug-13, David Christensen wrote:
Both bugs #16676[1] and #17141[2] illustrate that the combination of
SKIP LOCKED and FETCH FIRST WITH TIES break expectations when it comes
to rows returned to other sessions accessing the same row. Since this
situation is detectable from the syntax and hard to fix otherwise,
forbid for now, with the potential to fix in the future.
I think we should do this, given that it has show potential to bite
people. We should also add a small mentioned to this in the docs, as in
the attached.
What do others think?
--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
"Escucha y olvidarás; ve y recordarás; haz y entenderás" (Confucio)
Attachments:
ties-no-skip.patchtext/x-diff; charset=utf-8Download
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index fa676b1698..16bbab52c3 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -1515,7 +1515,8 @@ FETCH { FIRST | NEXT } [ <replaceable class="parameter">count</replaceable> ] {
The <literal>WITH TIES</literal> option is used to return any additional
rows that tie for the last place in the result set according to
the <literal>ORDER BY</literal> clause; <literal>ORDER BY</literal>
- is mandatory in this case.
+ is mandatory in this case, and <literal>SKIP LOCKED</literal> is
+ not allowed.
<literal>ROW</literal> and <literal>ROWS</literal> as well as
<literal>FIRST</literal> and <literal>NEXT</literal> are noise
words that don't influence the effects of these clauses.
On Mon, Aug 30, 2021 at 3:51 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2021-Aug-13, David Christensen wrote:
Both bugs #16676[1] and #17141[2] illustrate that the combination of
SKIP LOCKED and FETCH FIRST WITH TIES break expectations when it comes
to rows returned to other sessions accessing the same row. Since this
situation is detectable from the syntax and hard to fix otherwise,
forbid for now, with the potential to fix in the future.I think we should do this, given that it has show potential to bite
people. We should also add a small mentioned to this in the docs, as in
the attached.What do others think?
The patch I included had a doc mention in roughly the same place (at
least the same file), but I'm not tied to the wording I used (and I
may have missed more than one spot that might be appropriate). In any
case, +1.
David
On Mon, Aug 30, 2021 at 03:55:10PM -0500, David Christensen wrote:
On Mon, Aug 30, 2021 at 3:51 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
I think we should do this, given that it has show potential to bite
people. We should also add a small mentioned to this in the docs, as in
the attached.What do others think?
There is a CF entry for this patch:
https://commitfest.postgresql.org/34/3286/
Alvaro, are you planning to wrap that?
--
Michael
On 2021-Oct-01, Michael Paquier wrote:
On Mon, Aug 30, 2021 at 03:55:10PM -0500, David Christensen wrote:
On Mon, Aug 30, 2021 at 3:51 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
I think we should do this, given that it has show potential to bite
people. We should also add a small mentioned to this in the docs, as in
the attached.What do others think?
There is a CF entry for this patch:
https://commitfest.postgresql.org/34/3286/Alvaro, are you planning to wrap that?
I've had mixed feelings about this whole idea, but I think it's the
right thing to do. I'll try to get it pushed today.
--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
"I can't go to a restaurant and order food because I keep looking at the
fonts on the menu. Five minutes later I realize that it's also talking
about food" (Donald Knuth)
On 2021-Aug-13, David Christensen wrote:
Both bugs #16676[1] and #17141[2] illustrate that the combination of
SKIP LOCKED and FETCH FIRST WITH TIES break expectations when it comes
to rows returned to other sessions accessing the same row. Since this
situation is detectable from the syntax and hard to fix otherwise,
forbid for now, with the potential to fix in the future.
Thank you, pushed with minimal adjustment.
--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
"Digital and video cameras have this adjustment and film cameras don't for the
same reason dogs and cats lick themselves: because they can." (Ken Rockwell)
On Fri, Oct 01, 2021 at 06:33:01PM -0300, Alvaro Herrera wrote:
On 2021-Aug-13, David Christensen wrote:
Both bugs #16676[1] and #17141[2] illustrate that the combination of
SKIP LOCKED and FETCH FIRST WITH TIES break expectations when it comes
to rows returned to other sessions accessing the same row. Since this
situation is detectable from the syntax and hard to fix otherwise,
forbid for now, with the potential to fix in the future.Thank you, pushed with minimal adjustment.
BTW, I just marked this one as committed in CF app
--
Jaime Casanova
Director de Servicios Profesionales
SystemGuards - Consultores de PostgreSQL