From f1855169b20ecfa12016c89abac52913cedf6688 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 12 Mar 2025 11:02:33 +0100 Subject: [PATCH v21.1] Simplify and generalize PrepareSortSupportFromIndexRel() PrepareSortSupportFromIndexRel() was accepting btree strategy numbers purely for the purpose of comparing it later against btree strategies to determine if the sort direction was forward or reverse. Change that. Instead, pass a bool directly, to indicate the same without an unfortunate assumption that a strategy number refers specifically to a btree strategy. (This is similar in spirit to commits 0d2aa4d4937 and c594f1ad2ba.) (This could arguably be simplfied further by having the callers fill in ssup_reverse directly. But this way, it preserves consistency by having all PrepareSortSupport*() variants be responsible for filling in ssup_reverse.) Moreover, remove the hardcoded check against BTREE_AM_OID, and check against amcanorder instead, which is the actual requirement. Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com --- src/backend/access/nbtree/nbtsort.c | 7 +++---- src/backend/utils/sort/sortsupport.c | 15 ++++++--------- src/backend/utils/sort/tuplesortvariants.c | 14 ++++++-------- src/include/utils/sortsupport.h | 2 +- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c index fa336ba0096..3794cc924ad 100644 --- a/src/backend/access/nbtree/nbtsort.c +++ b/src/backend/access/nbtree/nbtsort.c @@ -1171,7 +1171,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2) { SortSupport sortKey = sortKeys + i; ScanKey scanKey = wstate->inskey->scankeys + i; - int16 strategy; + bool reverse; sortKey->ssup_cxt = CurrentMemoryContext; sortKey->ssup_collation = scanKey->sk_collation; @@ -1183,10 +1183,9 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2) Assert(sortKey->ssup_attno != 0); - strategy = (scanKey->sk_flags & SK_BT_DESC) != 0 ? - BTGreaterStrategyNumber : BTLessStrategyNumber; + reverse = (scanKey->sk_flags & SK_BT_DESC) != 0; - PrepareSortSupportFromIndexRel(wstate->index, strategy, sortKey); + PrepareSortSupportFromIndexRel(wstate->index, reverse, sortKey); } for (;;) diff --git a/src/backend/utils/sort/sortsupport.c b/src/backend/utils/sort/sortsupport.c index 6037031eaa3..0b4f08ed2ec 100644 --- a/src/backend/utils/sort/sortsupport.c +++ b/src/backend/utils/sort/sortsupport.c @@ -150,15 +150,15 @@ PrepareSortSupportFromOrderingOp(Oid orderingOp, SortSupport ssup) } /* - * Fill in SortSupport given an index relation, attribute, and strategy. + * Fill in SortSupport given an index relation and attribute. * * Caller must previously have zeroed the SortSupportData structure and then * filled in ssup_cxt, ssup_attno, ssup_collation, and ssup_nulls_first. This - * will fill in ssup_reverse (based on the supplied strategy), as well as the + * will fill in ssup_reverse (based on the supplied argument), as well as the * comparator function pointer. */ void -PrepareSortSupportFromIndexRel(Relation indexRel, int16 strategy, +PrepareSortSupportFromIndexRel(Relation indexRel, bool reverse, SortSupport ssup) { Oid opfamily = indexRel->rd_opfamily[ssup->ssup_attno - 1]; @@ -166,12 +166,9 @@ PrepareSortSupportFromIndexRel(Relation indexRel, int16 strategy, Assert(ssup->comparator == NULL); - if (indexRel->rd_rel->relam != BTREE_AM_OID) - elog(ERROR, "unexpected non-btree AM: %u", indexRel->rd_rel->relam); - if (strategy != BTGreaterStrategyNumber && - strategy != BTLessStrategyNumber) - elog(ERROR, "unexpected sort support strategy: %d", strategy); - ssup->ssup_reverse = (strategy == BTGreaterStrategyNumber); + if (!indexRel->rd_indam->amcanorder) + elog(ERROR, "unexpected non-amcanorder AM: %u", indexRel->rd_rel->relam); + ssup->ssup_reverse = reverse; FinishSortSupportFunction(opfamily, opcintype, ssup); } diff --git a/src/backend/utils/sort/tuplesortvariants.c b/src/backend/utils/sort/tuplesortvariants.c index eb8601e2257..4059af5bb71 100644 --- a/src/backend/utils/sort/tuplesortvariants.c +++ b/src/backend/utils/sort/tuplesortvariants.c @@ -329,7 +329,7 @@ tuplesort_begin_cluster(TupleDesc tupDesc, { SortSupport sortKey = base->sortKeys + i; ScanKey scanKey = indexScanKey->scankeys + i; - int16 strategy; + bool reverse; sortKey->ssup_cxt = CurrentMemoryContext; sortKey->ssup_collation = scanKey->sk_collation; @@ -341,10 +341,9 @@ tuplesort_begin_cluster(TupleDesc tupDesc, Assert(sortKey->ssup_attno != 0); - strategy = (scanKey->sk_flags & SK_BT_DESC) != 0 ? - BTGreaterStrategyNumber : BTLessStrategyNumber; + reverse = (scanKey->sk_flags & SK_BT_DESC) != 0; - PrepareSortSupportFromIndexRel(indexRel, strategy, sortKey); + PrepareSortSupportFromIndexRel(indexRel, reverse, sortKey); } pfree(indexScanKey); @@ -412,7 +411,7 @@ tuplesort_begin_index_btree(Relation heapRel, { SortSupport sortKey = base->sortKeys + i; ScanKey scanKey = indexScanKey->scankeys + i; - int16 strategy; + bool reverse; sortKey->ssup_cxt = CurrentMemoryContext; sortKey->ssup_collation = scanKey->sk_collation; @@ -424,10 +423,9 @@ tuplesort_begin_index_btree(Relation heapRel, Assert(sortKey->ssup_attno != 0); - strategy = (scanKey->sk_flags & SK_BT_DESC) != 0 ? - BTGreaterStrategyNumber : BTLessStrategyNumber; + reverse = (scanKey->sk_flags & SK_BT_DESC) != 0; - PrepareSortSupportFromIndexRel(indexRel, strategy, sortKey); + PrepareSortSupportFromIndexRel(indexRel, reverse, sortKey); } pfree(indexScanKey); diff --git a/src/include/utils/sortsupport.h b/src/include/utils/sortsupport.h index 51200db8ae9..b7abaf7802d 100644 --- a/src/include/utils/sortsupport.h +++ b/src/include/utils/sortsupport.h @@ -384,7 +384,7 @@ extern int ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup); /* Other functions in utils/sort/sortsupport.c */ extern void PrepareSortSupportComparisonShim(Oid cmpFunc, SortSupport ssup); extern void PrepareSortSupportFromOrderingOp(Oid orderingOp, SortSupport ssup); -extern void PrepareSortSupportFromIndexRel(Relation indexRel, int16 strategy, +extern void PrepareSortSupportFromIndexRel(Relation indexRel, bool reverse, SortSupport ssup); extern void PrepareSortSupportFromGistIndexRel(Relation indexRel, SortSupport ssup); -- 2.48.1