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://postgr.es/m/16676-fd62c3c835880da6@postgresql.org
[2] https://postgr.es/m/17141-913d78b9675aac8e@postgresql.org

Backpatch-through: 13, where WITH TIES was introduced
Author: David Christensen <[email protected]>
Discussion: https://postgr.es/m/CAOxo6XLPccCKru3xPMaYDpa+AXyPeWFs+SskrrL+HKwDjJnLhg@mail.gmail.com

diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index fa676b1698d4db2421a9dd1df43f14b378ba0545..16bbab52c3e6fe40347bbb029cb994cfb1429a3a 100644 (file)
--- 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.

diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 512b00bc656961a82255f112f144bcb9b4d324a8..fbbf769a8717d835fbb8e016f7fb4419dc114175 100644 (file)
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -185,7 +185,8 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
    if (concurrent && stmt->skipData)
        ereport(ERROR,
                (errcode(ERRCODE_SYNTAX_ERROR),
-                errmsg("CONCURRENTLY and WITH NO DATA options cannot be used together")));
+                errmsg("%s and %s options cannot be used together",
+                       "CONCURRENTLY", "WITH NO DATA")));
 
    /*
     * Check that everything is correct for a refresh. Problems at this point

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e3068a374ee42366a0a26374041044c5ab765305..08f1bf1031c96aab2f09fe463b9636ca9ce64cba 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -16816,6 +16816,21 @@ 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 *lock = lfirst_node(LockingClause, lc);
+
+               if (lock->waitPolicy == LockWaitSkip)
+                   ereport(ERROR,
+                           (errcode(ERRCODE_SYNTAX_ERROR),
+                            errmsg("%s and %s options cannot be used together",
+                                   "SKIP LOCKED", "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 b75afcc01a399db06e894ca089e307e9a3e183ac..8a98bbea8eb97ce6cc66526a106fda6210f1db24 100644 (file)
--- 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:  SKIP LOCKED and WITH TIES options cannot be used together
 -- 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 d2d4ef132df2f0a1e75dbdb8c3dd281b855dc852..6f0cda98701559d13d620d68855f24a15914f0ba 100644 (file)
--- 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