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, 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
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/a97d7401-e7c9-f771-6a00-037379f0a8bb%40gmail.com

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index d0dcbf318d5e490d0cad7897526c195632d154bf..cecee93ad8ea26b61361d394e1a6fe26d4e973af 100644 (file)
--- 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 a snapshot in the
+    * concurrent path.
+    */
+   if (concurrent)
+       PushActiveSnapshot(GetTransactionSnapshot());
+   else
+       Assert(HaveRegisteredOrActiveSnapshot());
+
    /*
     * 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 ec5f253b28fe32b38d13292ad47aa035c0eda6b0..130cebd658863243d6ecb46c7fa500c7945f9f26 100644 (file)
--- 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 f25723da92bddda6353af76dad6cbe889d400f6b..69becce19b1ebb0998c626a50d3ff33266119955 100644 (file)
--- 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;
+-- test that 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 5f1f4b80c95cf69f56c44b61c029c1402ed084b1..04834441db4ff5066adeb7707434de771c695f48 100644 (file)
--- 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;
+
+-- test that 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;