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)