From ad490b60a2e1b6fe05dee8a0bacf8aa5a944f6eb Mon Sep 17 00:00:00 2001 From: shihao zhong Date: Mon, 9 Nov 2026 23:03:56 -0500 Subject: [PATCH 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(). Since v17 such a state uses a Bump context, so non-assert builds can crash too. Fix by ending the sort state on rescan in that case, so that a new one is 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 613d7d1c349..87e2e9e8fa3 100644 --- a/src/backend/executor/nodeIncrementalSort.c +++ b/src/backend/executor/nodeIncrementalSort.c @@ -582,9 +582,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 @@ -1062,6 +1064,28 @@ ExecEndIncrementalSort(IncrementalSortState *node) ExecEndNode(outerPlanState(node)); } +/* + * resetSortState + * + * Reset a sort state for a rescan, or free it if it can't be used for a + * bounded sort when one is needed. + */ +static void +resetSortState(Tuplesortstate **state, bool bounded) +{ + if (*state == NULL) + return; + + if (bounded && + (TuplesortstateGetPublic(*state)->sortopt & TUPLESORT_ALLOWBOUNDED) == 0) + { + tuplesort_end(*state); + *state = NULL; + } + else + tuplesort_reset(*state); +} + void ExecReScanIncrementalSort(IncrementalSortState *node) { @@ -1098,16 +1122,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 but a sort state was created while it was not. Such a sort state + * 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 1e6e020fea8..b9fa68f8a6c 100644 --- a/src/test/regress/expected/incremental_sort.out +++ b/src/test/regress/expected/incremental_sort.out @@ -721,6 +721,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 bbe658a7588..736af88405e 100644 --- a/src/test/regress/sql/incremental_sort.sql +++ b/src/test/regress/sql/incremental_sort.sql @@ -162,6 +162,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)