From be86add263adbd106c283f65f7eb9cfb5449fa8a Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Tue, 27 Jan 2026 16:20:59 +0530 Subject: [PATCH v1] Refactor: Move CheckXidAlive check to table_beginscan for better performance Previously, the CheckXidAlive validation was performed within each table_scan_* function. This caused the check to be executed repeatedly for every tuple fetched, creating unnecessary overhead. Move the check to table_beginscan* so it is performed once per scan rather than once per row. Author: Dilip Kumar Reported-by: Andres Freund Suggested-by: Andres Freund, Amit Kapila --- src/backend/access/heap/heapam.c | 10 ----- src/backend/access/index/genam.c | 30 +++++++------- src/backend/access/table/tableam.c | 43 ++++++++++++++------ src/include/access/tableam.h | 65 +++++------------------------- 4 files changed, 56 insertions(+), 92 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index f30a56ecf55..ae31efe8c25 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -1421,16 +1421,6 @@ heap_getnext(TableScanDesc sscan, ScanDirection direction) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg_internal("only heap AM is supported"))); - /* - * We don't expect direct calls to heap_getnext with valid CheckXidAlive - * for catalog or regular tables. See detailed comments in xact.c where - * these variables are declared. Normally we have such a check at tableam - * level API but this is called from many places so we need to ensure it - * here. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected heap_getnext call during logical decoding"); - /* Note: no locking manipulations needed */ if (scan->rs_base.rs_flags & SO_ALLOW_PAGEMODE) diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c index a29be6f467b..5e89b86a62c 100644 --- a/src/backend/access/index/genam.c +++ b/src/backend/access/index/genam.c @@ -420,6 +420,14 @@ systable_beginscan(Relation heapRelation, sysscan->snapshot = NULL; } + /* + * If CheckXidAlive is set then set a flag to indicate that system table + * scan is in-progress. See detailed comments in xact.c where these + * variables are declared. + */ + if (TransactionIdIsValid(CheckXidAlive)) + bsysscan = true; + if (irel) { int i; @@ -468,14 +476,6 @@ systable_beginscan(Relation heapRelation, sysscan->iscan = NULL; } - /* - * If CheckXidAlive is set then set a flag to indicate that system table - * scan is in-progress. See detailed comments in xact.c where these - * variables are declared. - */ - if (TransactionIdIsValid(CheckXidAlive)) - bsysscan = true; - return sysscan; } @@ -707,13 +707,6 @@ systable_beginscan_ordered(Relation heapRelation, elog(ERROR, "column is not in index"); } - sysscan->iscan = index_beginscan(heapRelation, indexRelation, - snapshot, NULL, nkeys, 0); - index_rescan(sysscan->iscan, idxkey, nkeys, NULL, 0); - sysscan->scan = NULL; - - pfree(idxkey); - /* * If CheckXidAlive is set then set a flag to indicate that system table * scan is in-progress. See detailed comments in xact.c where these @@ -722,6 +715,13 @@ systable_beginscan_ordered(Relation heapRelation, if (TransactionIdIsValid(CheckXidAlive)) bsysscan = true; + sysscan->iscan = index_beginscan(heapRelation, indexRelation, + snapshot, NULL, nkeys, 0); + index_rescan(sysscan->iscan, idxkey, nkeys, NULL, 0); + sysscan->scan = NULL; + + pfree(idxkey); + return sysscan; } diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 87491796523..15a92c052d3 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -117,8 +117,8 @@ table_beginscan_catalog(Relation relation, int nkeys, ScanKeyData *key) Oid relid = RelationGetRelid(relation); Snapshot snapshot = RegisterSnapshot(GetCatalogSnapshot(relid)); - return relation->rd_tableam->scan_begin(relation, snapshot, nkeys, key, - NULL, flags); + return table_scan_begin_wrapper(relation, snapshot, nkeys, key, + NULL, flags); } @@ -184,7 +184,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc pscan) snapshot = SnapshotAny; } - return relation->rd_tableam->scan_begin(relation, snapshot, 0, NULL, + return table_scan_begin_wrapper(relation, snapshot, 0, NULL, pscan, flags); } @@ -214,8 +214,8 @@ table_beginscan_parallel_tidrange(Relation relation, snapshot = SnapshotAny; } - sscan = relation->rd_tableam->scan_begin(relation, snapshot, 0, NULL, - pscan, flags); + sscan = table_scan_begin_wrapper(relation, snapshot, 0, NULL, + pscan, flags); return sscan; } @@ -269,14 +269,6 @@ table_tuple_get_latest_tid(TableScanDesc scan, ItemPointer tid) Relation rel = scan->rs_rd; const TableAmRoutine *tableam = rel->rd_tableam; - /* - * We don't expect direct calls to table_tuple_get_latest_tid with valid - * CheckXidAlive for catalog or regular tables. See detailed comments in - * xact.c where these variables are declared. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected table_tuple_get_latest_tid call during logical decoding"); - /* * Since this can be called with user-supplied TID, don't trust the input * too much. @@ -829,3 +821,28 @@ table_block_relation_estimate_size(Relation rel, int32 *attr_widths, else *allvisfrac = (double) relallvisible / curpages; } + +/* + * table_scan_begin_wrapper + * + * A wrapper around the Table Access Method scan_begin callback. This handles + * centralized error checking—specifically ensuring we aren't performing + * table scan while CheckXidAlive is valid. This state is reserved for + * specific logical decoding operations where direct relation scanning is + * prohibited. + */ +TableScanDesc +table_scan_begin_wrapper(Relation rel, Snapshot snapshot, int nkeys, + ScanKeyData *key, ParallelTableScanDesc pscan, + uint32 flags) +{ + /* + * We don't expect direct calls to this function with valid CheckXidAlive + * for catalog or regular tables. See detailed comments in xact.c where + * these variables are declared. + */ + if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) + elog(ERROR, "unexpected table_scan_begin_wrapper call during logical decoding"); + + return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, pscan, flags); +} diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index e2ec5289d4d..d4a18b93c36 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -862,6 +862,10 @@ extern const TupleTableSlotOps *table_slot_callbacks(Relation relation); */ extern TupleTableSlot *table_slot_create(Relation relation, List **reglist); +extern TableScanDesc table_scan_begin_wrapper(Relation rel, Snapshot snapshot, + int nkeys, ScanKeyData *key, + ParallelTableScanDesc pscan, + uint32 flags); /* ---------------------------------------------------------------------------- * Table scan functions. @@ -879,7 +883,7 @@ table_beginscan(Relation rel, Snapshot snapshot, uint32 flags = SO_TYPE_SEQSCAN | SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE; - return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); + return table_scan_begin_wrapper(rel, snapshot, nkeys, key, NULL, flags); } /* @@ -908,7 +912,7 @@ table_beginscan_strat(Relation rel, Snapshot snapshot, if (allow_sync) flags |= SO_ALLOW_SYNC; - return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); + return table_scan_begin_wrapper(rel, snapshot, nkeys, key, NULL, flags); } /* @@ -923,8 +927,7 @@ table_beginscan_bm(Relation rel, Snapshot snapshot, { uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE; - return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, - NULL, flags); + return table_scan_begin_wrapper(rel, snapshot, nkeys, key, NULL, flags); } /* @@ -949,7 +952,7 @@ table_beginscan_sampling(Relation rel, Snapshot snapshot, if (allow_pagemode) flags |= SO_ALLOW_PAGEMODE; - return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); + return table_scan_begin_wrapper(rel, snapshot, nkeys, key, NULL, flags); } /* @@ -962,7 +965,7 @@ table_beginscan_tid(Relation rel, Snapshot snapshot) { uint32 flags = SO_TYPE_TIDSCAN; - return rel->rd_tableam->scan_begin(rel, snapshot, 0, NULL, NULL, flags); + return table_scan_begin_wrapper(rel, snapshot, 0, NULL, NULL, flags); } /* @@ -975,7 +978,7 @@ table_beginscan_analyze(Relation rel) { uint32 flags = SO_TYPE_ANALYZE; - return rel->rd_tableam->scan_begin(rel, NULL, 0, NULL, NULL, flags); + return table_scan_begin_wrapper(rel, NULL, 0, NULL, NULL, flags); } /* @@ -1025,14 +1028,6 @@ table_scan_getnextslot(TableScanDesc sscan, ScanDirection direction, TupleTableS Assert(direction == ForwardScanDirection || direction == BackwardScanDirection); - /* - * We don't expect direct calls to table_scan_getnextslot with valid - * CheckXidAlive for catalog or regular tables. See detailed comments in - * xact.c where these variables are declared. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected table_scan_getnextslot call during logical decoding"); - return sscan->rs_rd->rd_tableam->scan_getnextslot(sscan, direction, slot); } @@ -1053,7 +1048,7 @@ table_beginscan_tidrange(Relation rel, Snapshot snapshot, TableScanDesc sscan; uint32 flags = SO_TYPE_TIDRANGESCAN | SO_ALLOW_PAGEMODE; - sscan = rel->rd_tableam->scan_begin(rel, snapshot, 0, NULL, NULL, flags); + sscan = table_scan_begin_wrapper(rel, snapshot, 0, NULL, NULL, flags); /* Set the range of TIDs to scan */ sscan->rs_rd->rd_tableam->scan_set_tidrange(sscan, mintid, maxtid); @@ -1219,14 +1214,6 @@ table_index_fetch_tuple(struct IndexFetchTableData *scan, TupleTableSlot *slot, bool *call_again, bool *all_dead) { - /* - * We don't expect direct calls to table_index_fetch_tuple with valid - * CheckXidAlive for catalog or regular tables. See detailed comments in - * xact.c where these variables are declared. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected table_index_fetch_tuple call during logical decoding"); - return scan->rel->rd_tableam->index_fetch_tuple(scan, tid, snapshot, slot, call_again, all_dead); @@ -1265,14 +1252,6 @@ table_tuple_fetch_row_version(Relation rel, Snapshot snapshot, TupleTableSlot *slot) { - /* - * We don't expect direct calls to table_tuple_fetch_row_version with - * valid CheckXidAlive for catalog or regular tables. See detailed - * comments in xact.c where these variables are declared. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected table_tuple_fetch_row_version call during logical decoding"); - return rel->rd_tableam->tuple_fetch_row_version(rel, tid, snapshot, slot); } @@ -1947,14 +1926,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, uint64 *lossy_pages, uint64 *exact_pages) { - /* - * We don't expect direct calls to table_scan_bitmap_next_tuple with valid - * CheckXidAlive for catalog or regular tables. See detailed comments in - * xact.c where these variables are declared. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); - return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, slot, recheck, @@ -1975,13 +1946,6 @@ static inline bool table_scan_sample_next_block(TableScanDesc scan, SampleScanState *scanstate) { - /* - * We don't expect direct calls to table_scan_sample_next_block with valid - * CheckXidAlive for catalog or regular tables. See detailed comments in - * xact.c where these variables are declared. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected table_scan_sample_next_block call during logical decoding"); return scan->rs_rd->rd_tableam->scan_sample_next_block(scan, scanstate); } @@ -1998,13 +1962,6 @@ table_scan_sample_next_tuple(TableScanDesc scan, SampleScanState *scanstate, TupleTableSlot *slot) { - /* - * We don't expect direct calls to table_scan_sample_next_tuple with valid - * CheckXidAlive for catalog or regular tables. See detailed comments in - * xact.c where these variables are declared. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected table_scan_sample_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_sample_next_tuple(scan, scanstate, slot); } -- 2.49.0