From 3be62fac910e930f5635193ac60d1536b17e0d6d Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 23 Sep 2024 10:33:58 -0500 Subject: [PATCH v3 1/1] Ensure we have a snapshot when updating pg_index entries. Creating, reindexing, and dropping an index concurrently could entail accessing pg_index's TOAST table, which was recently added in commit b52c4fc3c0. These code paths start and commit their own transactions internally, but they do not always set an active snapshot. This rightfully leads to assertion failures and ERRORs when trying to access pg_index's TOAST table, such as the following: ERROR: cannot fetch toast data without an active snapshot To fix, push an active snapshot just before each section of code that might require accessing pg_index's TOAST table, and pop it shortly afterwards. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/a97d7401-e7c9-f771-6a00-037379f0a8bb%40gmail.com --- src/backend/catalog/index.c | 21 +++++++++++++++++++++ src/backend/commands/indexcmds.c | 24 ++++++++++++++++++++++++ src/test/regress/expected/indexing.out | 15 +++++++++++++++ src/test/regress/sql/indexing.sql | 16 ++++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index b2b3ecb524..09d79b26b8 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2276,9 +2276,17 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) */ WaitForLockers(heaplocktag, AccessExclusiveLock, true); + /* + * Updating pg_index might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* Finish invalidation of index and mark it as dead */ index_concurrently_set_dead(heapId, indexId); + PopActiveSnapshot(); + /* * Again, commit the transaction to make the pg_index update visible * to other sessions. @@ -2326,6 +2334,16 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) RelationForgetRelation(indexId); + /* + * Updating pg_index might involve TOAST table access, so ensure we have a + * valid snapshot. We only expect to get here without an active snapshot + * in the concurrent path. + */ + if (concurrent) + PushActiveSnapshot(GetTransactionSnapshot()); + else + Assert(ActiveSnapshotSet()); + /* * fix INDEX relation, and check for expressional index */ @@ -2343,6 +2361,9 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) ReleaseSysCache(tuple); table_close(indexRelation, RowExclusiveLock); + if (concurrent) + PopActiveSnapshot(); + /* * if it has any expression columns, we might have stored statistics about * them. diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index f99c2d2dee..7d41cb73ab 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1798,11 +1798,19 @@ DefineIndex(Oid tableId, PROGRESS_CREATEIDX_PHASE_WAIT_3); WaitForOlderSnapshots(limitXmin, true); + /* + * Updating pg_index might involve TOAST table access, so ensure we have a + * valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Index can now be marked valid -- update its pg_index entry */ index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID); + PopActiveSnapshot(); + /* * The pg_index update will cause backends (including this one) to update * relcache entries for the index itself, but we should also send a @@ -4256,12 +4264,20 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein get_rel_namespace(oldidx->tableId), false); + /* + * Updating pg_index might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Swap old index with the new one. This also marks the new one as * valid and the old one as not valid. */ index_concurrently_swap(newidx->indexId, oldidx->indexId, oldName); + PopActiveSnapshot(); + /* * Invalidate the relcache for the table, so that after this commit * all sessions will refresh any cached plans that might reference the @@ -4312,7 +4328,15 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein */ CHECK_FOR_INTERRUPTS(); + /* + * Updating pg_index might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + index_concurrently_set_dead(oldidx->tableId, oldidx->indexId); + + PopActiveSnapshot(); } /* Commit this transaction to make the updates visible. */ diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index f25723da92..49f92fe81f 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -1640,3 +1640,18 @@ select indexrelid::regclass, indisvalid, indisreplident, (3 rows) drop table parted_replica_tab; +-- indexing commands work with TOASTed values in pg_index +create table test_pg_index_toast_table (a int); +create or replace function test_pg_index_toast_func (a int, b int[]) + returns bool as $$ select true $$ language sql immutable; +select array_agg(n) b from generate_series(1, 10000) n \gset +create index concurrently test_pg_index_toast_index + on test_pg_index_toast_table (test_pg_index_toast_func(a, :'b')); +reindex index concurrently test_pg_index_toast_index; +drop index concurrently test_pg_index_toast_index; +create index test_pg_index_toast_index + on test_pg_index_toast_table (test_pg_index_toast_func(a, :'b')); +reindex index test_pg_index_toast_index; +drop index test_pg_index_toast_index; +drop function test_pg_index_toast_func; +drop table test_pg_index_toast_table; diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index 5f1f4b80c9..8c07d49d46 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -919,3 +919,19 @@ select indexrelid::regclass, indisvalid, indisreplident, where indexrelid::regclass::text like 'parted_replica%' order by indexrelid::regclass::text collate "C"; drop table parted_replica_tab; + +-- indexing commands work with TOASTed values in pg_index +create table test_pg_index_toast_table (a int); +create or replace function test_pg_index_toast_func (a int, b int[]) + returns bool as $$ select true $$ language sql immutable; +select array_agg(n) b from generate_series(1, 10000) n \gset +create index concurrently test_pg_index_toast_index + on test_pg_index_toast_table (test_pg_index_toast_func(a, :'b')); +reindex index concurrently test_pg_index_toast_index; +drop index concurrently test_pg_index_toast_index; +create index test_pg_index_toast_index + on test_pg_index_toast_table (test_pg_index_toast_func(a, :'b')); +reindex index test_pg_index_toast_index; +drop index test_pg_index_toast_index; +drop function test_pg_index_toast_func; +drop table test_pg_index_toast_table; -- 2.39.5 (Apple Git-154)