From 672c68c05cdea4d46624dc7ceb01b1203c45d121 Mon Sep 17 00:00:00 2001 From: Hayato Kuroda Date: Thu, 1 Oct 2026 16:59:00 +0900 Subject: [PATCH v2-HEAD] Track the depth of scans of system tables --- contrib/test_decoding/expected/stream.out | 45 +++++++++++++++- contrib/test_decoding/sql/stream.sql | 34 ++++++++++++- src/backend/access/index/genam.c | 62 +++++++++++++---------- src/backend/replication/logical/logical.c | 3 +- src/include/access/genam.h | 2 + src/include/access/tableam.h | 8 +-- src/include/access/xact.h | 2 +- 7 files changed, 122 insertions(+), 34 deletions(-) diff --git a/contrib/test_decoding/expected/stream.out b/contrib/test_decoding/expected/stream.out index 0ec5c933610..baf02ac6297 100644 --- a/contrib/test_decoding/expected/stream.out +++ b/contrib/test_decoding/expected/stream.out @@ -134,6 +134,49 @@ SELECT count(*) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, (1 row) RESET debug_logical_replication_streaming; +-- Create a table with a large constraint whose conbin value is stored out of +-- line. +DO $$ +DECLARE + large_literal text; +BEGIN + SELECT string_agg(md5(i::text), '') INTO large_literal FROM generate_series(1, 100) i; + + EXECUTE format( + 'CREATE TABLE nested_sys_scan_test ( + v text, + pad text, + CONSTRAINT a_big CHECK (v <> %L), + CONSTRAINT z_after CHECK (true))', + large_literal); +END +$$; +-- consume DDL +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + data +------ +(0 rows) + +-- Verify that detoasting catalog data in a nested system table scan does not +-- prevent the outer scan from fetching subsequent tuples. +SET debug_logical_replication_streaming = immediate; +BEGIN; +INSERT INTO nested_sys_scan_test VALUES ('y', repeat('x', 200)); +CHECKPOINT; +SELECT count(*) > 0 AS streamed FROM pg_logical_slot_peek_changes('regression_slot', NULL, NULL, 'stream-changes', '1'); + streamed +---------- + t +(1 row) + +COMMIT; +RESET debug_logical_replication_streaming; +SELECT count(*) > 0 AS streamed FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'stream-changes', '1'); + streamed +---------- + t +(1 row) + -- bug #19616 -- -- An aborted top-level transaction that is discarded at eviction must not @@ -157,7 +200,7 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc COMMIT (3 rows) -DROP TABLE stream_test; +DROP TABLE stream_test, nested_sys_scan_test; SELECT pg_drop_replication_slot('regression_slot'); pg_drop_replication_slot -------------------------- diff --git a/contrib/test_decoding/sql/stream.sql b/contrib/test_decoding/sql/stream.sql index 5e45a8e9b64..4e5a176aab0 100644 --- a/contrib/test_decoding/sql/stream.sql +++ b/contrib/test_decoding/sql/stream.sql @@ -65,6 +65,38 @@ COMMIT; SELECT count(*) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1', 'stream-changes', '1'); RESET debug_logical_replication_streaming; +-- Create a table with a large constraint whose conbin value is stored out of +-- line. +DO $$ +DECLARE + large_literal text; +BEGIN + SELECT string_agg(md5(i::text), '') INTO large_literal FROM generate_series(1, 100) i; + + EXECUTE format( + 'CREATE TABLE nested_sys_scan_test ( + v text, + pad text, + CONSTRAINT a_big CHECK (v <> %L), + CONSTRAINT z_after CHECK (true))', + large_literal); +END +$$; + +-- consume DDL +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + +-- Verify that detoasting catalog data in a nested system table scan does not +-- prevent the outer scan from fetching subsequent tuples. +SET debug_logical_replication_streaming = immediate; +BEGIN; +INSERT INTO nested_sys_scan_test VALUES ('y', repeat('x', 200)); +CHECKPOINT; +SELECT count(*) > 0 AS streamed FROM pg_logical_slot_peek_changes('regression_slot', NULL, NULL, 'stream-changes', '1'); +COMMIT; +RESET debug_logical_replication_streaming; +SELECT count(*) > 0 AS streamed FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'stream-changes', '1'); + -- bug #19616 -- -- An aborted top-level transaction that is discarded at eviction must not @@ -82,5 +114,5 @@ ROLLBACK; INSERT INTO stream_test VALUES ('after-abort'); SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); -DROP TABLE stream_test; +DROP TABLE stream_test, nested_sys_scan_test; SELECT pg_drop_replication_slot('regression_slot'); diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c index 98ac5bc79c9..05f2d43d939 100644 --- a/src/backend/access/index/genam.c +++ b/src/backend/access/index/genam.c @@ -37,6 +37,38 @@ #include "utils/ruleutils.h" #include "utils/snapmgr.h" +/* Number of nested systable scans while decoding in-progress transactions. */ +int sysscan_depth = 0; + +/* + * System table scans can be nested while CheckXidAlive is valid. A simple + * boolean is insufficient because ending an inner scan must not clear the flag + * while an outer scan is still active. + */ +static inline void +IncrementSysScanDepth(void) +{ + if (!TransactionIdIsValid(CheckXidAlive)) + return; + + sysscan_depth++; +} + +static inline void +DecrementSysScanDepth(void) +{ + if (!TransactionIdIsValid(CheckXidAlive)) + return; + + Assert(sysscan_depth > 0); + sysscan_depth--; +} + +void +ResetSysScanDepth(void) +{ + sysscan_depth = 0; +} /* ---------------------------------------------------------------- * general access method routines @@ -432,13 +464,7 @@ 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; + IncrementSysScanDepth(); if (irel) { @@ -633,12 +659,7 @@ systable_endscan(SysScanDesc sysscan) if (sysscan->snapshot) UnregisterSnapshot(sysscan->snapshot); - /* - * Reset the bsysscan flag at the end of the systable scan. See detailed - * comments in xact.c where these variables are declared. - */ - if (TransactionIdIsValid(CheckXidAlive)) - bsysscan = false; + DecrementSysScanDepth(); pfree(sysscan); } @@ -721,13 +742,7 @@ systable_beginscan_ordered(Relation heapRelation, elog(ERROR, "column is not in index"); } - /* - * 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; + IncrementSysScanDepth(); sysscan->iscan = index_beginscan(heapRelation, indexRelation, false, snapshot, NULL, nkeys, 0, @@ -782,12 +797,7 @@ systable_endscan_ordered(SysScanDesc sysscan) if (sysscan->snapshot) UnregisterSnapshot(sysscan->snapshot); - /* - * Reset the bsysscan flag at the end of the systable scan. See detailed - * comments in xact.c where these variables are declared. - */ - if (TransactionIdIsValid(CheckXidAlive)) - bsysscan = false; + DecrementSysScanDepth(); pfree(sysscan); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 98e5f1dd8f9..421d7d0d6b5 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -28,6 +28,7 @@ #include "postgres.h" +#include "access/genam.h" #include "access/xact.h" #include "access/xlog_internal.h" #include "access/xlogutils.h" @@ -2030,7 +2031,7 @@ void ResetLogicalStreamingState(void) { CheckXidAlive = InvalidTransactionId; - bsysscan = false; + ResetSysScanDepth(); } /* diff --git a/src/include/access/genam.h b/src/include/access/genam.h index 1bfdc1ef6b1..c5b9a457fb2 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -248,4 +248,6 @@ extern void systable_inplace_update_begin(Relation relation, extern void systable_inplace_update_finish(void *state, HeapTuple tuple); extern void systable_inplace_update_cancel(void *state); +extern void ResetSysScanDepth(void); + #endif /* GENAM_H */ diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index ea3f2a6be99..692f70adc90 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -927,7 +927,7 @@ table_beginscan_common(Relation rel, Snapshot snapshot, int nkeys, * via systable_beginscan() et al. See detailed comments in xact.c where * these variables are declared. */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) + if (unlikely(TransactionIdIsValid(CheckXidAlive) && !sysscan_depth)) elog(ERROR, "scan started during logical decoding"); return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, pscan, flags); @@ -1251,7 +1251,7 @@ table_index_scan_begin(IndexScanDesc scan, uint32 flags) * via systable_beginscan() et al. See detailed comments in xact.c where * these variables are declared. */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) + if (unlikely(TransactionIdIsValid(CheckXidAlive) && !sysscan_depth)) elog(ERROR, "scan started during logical decoding"); scan->heapRelation->rd_tableam->index_scan_begin(scan, flags); @@ -1344,7 +1344,7 @@ table_fetch_tid(Relation rel, * CheckXidAlive for catalog or regular tables. See detailed comments in * xact.c where these variables are declared. */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) + if (unlikely(TransactionIdIsValid(CheckXidAlive) && !sysscan_depth)) elog(ERROR, "unexpected table_fetch_tid call during logical decoding"); return rel->rd_tableam->fetch_tid(rel, tid, snapshot, all_dead); @@ -1369,7 +1369,7 @@ table_tuple_fetch_row_version(Relation rel, * valid CheckXidAlive for catalog or regular tables. See detailed * comments in xact.c where these variables are declared. */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) + if (unlikely(TransactionIdIsValid(CheckXidAlive) && !sysscan_depth)) elog(ERROR, "unexpected table_tuple_fetch_row_version call during logical decoding"); return rel->rd_tableam->tuple_fetch_row_version(rel, tid, snapshot, slot); diff --git a/src/include/access/xact.h b/src/include/access/xact.h index a8cbdf247c8..b28fe967589 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -85,7 +85,7 @@ extern PGDLLIMPORT int synchronous_commit; /* used during logical streaming of a transaction */ extern PGDLLIMPORT TransactionId CheckXidAlive; -extern PGDLLIMPORT bool bsysscan; +extern PGDLLIMPORT int sysscan_depth; /* * Miscellaneous flag bits to record events which occur on the top level -- 2.52.0