v1-0001-Small-cleanup-for-incremental-sort-bounded-mode-t.patch

From 746c533c1f56a286d21f241d1c88c48b4227bf3f Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Wed, 10 Feb 2021 17:35:41 -0500
Subject: [PATCH v1] Small cleanup for incremental sort bounded mode transition
 bug

- Add further comments explaining why the test case triggers the issue.
- Guard against the regression test changing plans.
- Simplify bug fix itself.
---
 src/backend/executor/nodeIncrementalSort.c     |  9 +--------
 src/test/regress/expected/incremental_sort.out | 15 +++++++++++++++
 src/test/regress/sql/incremental_sort.sql      |  4 ++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c
index 82fa800cb1..9da00a0d21 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -345,8 +345,6 @@ switchToPresortedPrefixMode(PlanState *pstate)
 	 */
 	for (;;)
 	{
-		lastTuple = node->n_fullsort_remaining - nTuples == 1;
-
 		/*
 		 * When we encounter multiple prefix key groups inside the full sort
 		 * tuplesort we have to carry over the last read tuple into the next
@@ -395,17 +393,12 @@ switchToPresortedPrefixMode(PlanState *pstate)
 				 */
 				ExecClearTuple(node->group_pivot);
 
-				/*
-				 * Also make sure we take the didn't-consume-all-the-tuples
-				 * path below, even if this happened to be the last tuple of
-				 * the batch.
-				 */
-				lastTuple = false;
 				break;
 			}
 		}
 
 		firstTuple = false;
+		lastTuple = node->n_fullsort_remaining - nTuples == 0;
 
 		/*
 		 * If we've copied all of the tuples from the full sort state into the
diff --git a/src/test/regress/expected/incremental_sort.out b/src/test/regress/expected/incremental_sort.out
index d574583844..68ca321163 100644
--- a/src/test/regress/expected/incremental_sort.out
+++ b/src/test/regress/expected/incremental_sort.out
@@ -676,6 +676,21 @@ select * from (select * from t order by a) s order by a, b limit 70;
 (70 rows)
 
 -- Checks case where we hit a group boundary at the last tuple of a batch.
+-- Because the full sort state is bounded, we scan 64 tuples (the mode
+-- transition point) but only retain 5. Thus when we transition modes, all
+-- tuples in the full sort state have different prefix keys.
+explain (costs off) select * from (select * from t order by a) s order by a, b limit 5;
+           QUERY PLAN            
+---------------------------------
+ Limit
+   ->  Incremental Sort
+         Sort Key: t.a, t.b
+         Presorted Key: t.a
+         ->  Sort
+               Sort Key: t.a
+               ->  Seq Scan on t
+(7 rows)
+
 select * from (select * from t order by a) s order by a, b limit 5;
  a | b 
 ---+---
diff --git a/src/test/regress/sql/incremental_sort.sql b/src/test/regress/sql/incremental_sort.sql
index 9965fcd777..81429164d4 100644
--- a/src/test/regress/sql/incremental_sort.sql
+++ b/src/test/regress/sql/incremental_sort.sql
@@ -150,6 +150,10 @@ analyze t;
 explain (costs off) select * from (select * from t order by a) s order by a, b limit 70;
 select * from (select * from t order by a) s order by a, b limit 70;
 -- Checks case where we hit a group boundary at the last tuple of a batch.
+-- Because the full sort state is bounded, we scan 64 tuples (the mode
+-- transition point) but only retain 5. Thus when we transition modes, all
+-- tuples in the full sort state have different prefix keys.
+explain (costs off) select * from (select * from t order by a) s order by a, b limit 5;
 select * from (select * from t order by a) s order by a, b limit 5;
 
 -- Test rescan.
-- 
2.17.1