From 1a0ca974bf427ed2d07b2c9dee35eae2d7bf4716 Mon Sep 17 00:00:00 2001 From: shihao zhong Date: Tue, 10 Nov 2026 20:41:27 -0500 Subject: [PATCH REL_15 v1] Fix Incremental Sort rescan that becomes bounded Incremental Sort reuses its sort states on rescan, but they only allow a bound if the node was bounded when they were created. A rescan that sets a bound then fails an assertion in tuplesort_set_bound(). Fix by ending the sort states on every bounded rescan, so that new ones are created. Backpatch-through: 15 --- src/backend/executor/nodeIncrementalSort.c | 43 ++++++++++++++----- .../regress/expected/incremental_sort.out | 34 +++++++++++++++ src/test/regress/sql/incremental_sort.sql | 12 ++++++ 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c index 7c0a80858ec..e18464deb35 100644 --- a/src/backend/executor/nodeIncrementalSort.c +++ b/src/backend/executor/nodeIncrementalSort.c @@ -597,9 +597,11 @@ ExecIncrementalSort(PlanState *pstate) * isCurrentGroup(). It's correct to do this along with the * initial initialization for the full sort state (and not for the * prefix sort state) since we always load the full sort state - * first. + * first. A rescan may have freed the full sort state, but the + * presorted column info can be kept. */ - preparePresortedCols(node); + if (node->presorted_keys == NULL) + preparePresortedCols(node); /* * Since we optimize small prefix key groups by accumulating a @@ -1109,6 +1111,28 @@ ExecEndIncrementalSort(IncrementalSortState *node) SO_printf("ExecEndIncrementalSort: sort node shutdown\n"); } +/* + * resetSortState + * + * Reset a sort state for a rescan, or free it if the node is bounded. We + * don't know here whether the sort state was created with + * TUPLESORT_ALLOWBOUNDED, so a bounded rescan always gets a new one. + */ +static void +resetSortState(Tuplesortstate **state, bool bounded) +{ + if (*state == NULL) + return; + + if (bounded) + { + tuplesort_end(*state); + *state = NULL; + } + else + tuplesort_reset(*state); +} + void ExecReScanIncrementalSort(IncrementalSortState *node) { @@ -1145,16 +1169,13 @@ ExecReScanIncrementalSort(IncrementalSortState *node) /* * If we've set up either of the sort states yet, we need to reset them. - * We could end them and null out the pointers, but there's no reason to - * repay the setup cost, and because ExecIncrementalSort guards presorted - * column functions by checking to see if the full sort state has been - * initialized yet, setting the sort states to null here might actually - * cause a leak. + * There's no reason to repay the setup cost, unless the node is bounded + * now. The sort state may have been created while it was not, and then + * it does not support bounded sorts, so free it and let + * ExecIncrementalSort create a new one. */ - if (node->fullsort_state != NULL) - tuplesort_reset(node->fullsort_state); - if (node->prefixsort_state != NULL) - tuplesort_reset(node->prefixsort_state); + resetSortState(&node->fullsort_state, node->bounded); + resetSortState(&node->prefixsort_state, node->bounded); /* * If chgParam of subnode is not null, then the plan will be re-scanned by diff --git a/src/test/regress/expected/incremental_sort.out b/src/test/regress/expected/incremental_sort.out index 0a631124c22..193155f2a30 100644 --- a/src/test/regress/expected/incremental_sort.out +++ b/src/test/regress/expected/incremental_sort.out @@ -732,6 +732,40 @@ select * from t left join (select * from (select * from t order by a) v order by 2 | 2 | 2 | 2 (2 rows) +rollback; +-- Test a bounded rescan after an unbounded first scan. +begin; +set local enable_seqscan = off; +set local enable_bitmapscan = off; +explain (costs off) +select o.n, count(*) from (values (null::int), (3)) o(n), + lateral (select unique1 from tenk1 where hundred < 2 order by hundred, unique1 limit o.n) s +group by o.n; + QUERY PLAN +----------------------------------------------------------------------- + GroupAggregate + Group Key: "*VALUES*".column1 + -> Sort + Sort Key: "*VALUES*".column1 + -> Nested Loop + -> Values Scan on "*VALUES*" + -> Limit + -> Incremental Sort + Sort Key: tenk1.hundred, tenk1.unique1 + Presorted Key: tenk1.hundred + -> Index Scan using tenk1_hundred on tenk1 + Index Cond: (hundred < 2) +(12 rows) + +select o.n, count(*) from (values (null::int), (3)) o(n), + lateral (select unique1 from tenk1 where hundred < 2 order by hundred, unique1 limit o.n) s +group by o.n; + n | count +---+------- + 3 | 3 + | 200 +(2 rows) + rollback; -- Test EXPLAIN ANALYZE with both fullsort and presorted groups. select explain_analyze_without_memory('select * from (select * from t order by a) s order by a, b limit 70'); diff --git a/src/test/regress/sql/incremental_sort.sql b/src/test/regress/sql/incremental_sort.sql index 284a354dbb7..b5e7c8709c2 100644 --- a/src/test/regress/sql/incremental_sort.sql +++ b/src/test/regress/sql/incremental_sort.sql @@ -167,6 +167,18 @@ set local enable_sort = off; explain (costs off) select * from t left join (select * from (select * from t order by a) v order by a, b) s on s.a = t.a where t.a in (1, 2); select * from t left join (select * from (select * from t order by a) v order by a, b) s on s.a = t.a where t.a in (1, 2); rollback; +-- Test a bounded rescan after an unbounded first scan. +begin; +set local enable_seqscan = off; +set local enable_bitmapscan = off; +explain (costs off) +select o.n, count(*) from (values (null::int), (3)) o(n), + lateral (select unique1 from tenk1 where hundred < 2 order by hundred, unique1 limit o.n) s +group by o.n; +select o.n, count(*) from (values (null::int), (3)) o(n), + lateral (select unique1 from tenk1 where hundred < 2 order by hundred, unique1 limit o.n) s +group by o.n; +rollback; -- Test EXPLAIN ANALYZE with both fullsort and presorted groups. select explain_analyze_without_memory('select * from (select * from t order by a) s order by a, b limit 70'); select jsonb_pretty(explain_analyze_inc_sort_nodes_without_memory('select * from (select * from t order by a) s order by a, b limit 70')); -- 2.37.1 (Apple Git-137.1)