Error out if SKIP LOCKED and WITH TIES are both specified

alvherre · alvherre · commit 170206e458bc · 2021-10-01T18:29:18.000-03:00
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 <david.christensen@crunchydata.com> 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
@@ -1454,7 +1454,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
@@ -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
@@ -16163,6 +16163,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
@@ -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
@@ -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
