Make index_set_state_flags() transactional

3c84046 is the original commit that introduced index_set_state_flags(),
where the presence of SnapshotNow made necessary the use of an in-place
update.  SnapshotNow has been removed in 813fb03, so there is no actual
reasons to not make this operation transactional.

As reported by Andrey, it is possible to trigger the assertion of this
routine expecting no transactional updates when switching the pg_index
state flags, using a predicate mark as immutable but calling stable or
volatile functions.  83158f7 has been around for a couple of months on
HEAD now with no issues found related to it, so it looks safe enough for
a backpatch.

Reported-by: Andrey Lepikhov
Author: Michael Paquier
Reviewed-by: Anastasia Lubennikova
Discussion: https://postgr.es/m/20200903080440[email protected]
Discussion: https://postgr.es/m/9b905019-5297-7372-0ad2-e1a4bb66a719@postgrespro.ru
Backpatch-through: 9.6

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f1fe529981c0e19cc14f2802720cd28748bbe33e..d7d89a38bb7676b65926cdcf112f5c76f1c1219e 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3281,18 +3281,10 @@ validate_index_heapscan(Relation heapRelation,
  * index_set_state_flags - adjust pg_index state flags
  *
  * This is used during CREATE/DROP INDEX CONCURRENTLY to adjust the pg_index
- * flags that denote the index's state.  Because the update is not
- * transactional and will not roll back on error, this must only be used as
- * the last step in a transaction that has not made any transactional catalog
- * updates!
+ * flags that denote the index's state.
  *
- * Note that heap_inplace_update does send a cache inval message for the
+ * Note that simple_heap_update() sends a cache invalidation message for the
  * tuple, so other sessions will hear about the update as soon as we commit.
- *
- * NB: In releases prior to PostgreSQL 9.4, the use of a non-transactional
- * update here would have been unsafe; now that MVCC rules apply even for
- * system catalog scans, we could potentially use a transactional update here
- * instead.
  */
 void
 index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
@@ -3301,9 +3293,6 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
    HeapTuple   indexTuple;
    Form_pg_index indexForm;
 
-   /* Assert that current xact hasn't done any transactional updates */
-   Assert(GetTopTransactionIdIfAny() == InvalidTransactionId);
-
    /* Open pg_index and fetch a writable copy of the index's tuple */
    pg_index = heap_open(IndexRelationId, RowExclusiveLock);
 
@@ -3362,8 +3351,9 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
            break;
    }
 
-   /* ... and write it back in-place */
-   heap_inplace_update(pg_index, indexTuple);
+   /* ... and update it */
+   simple_heap_update(pg_index, &indexTuple->t_self, indexTuple);
+   CatalogUpdateIndexes(pg_index, indexTuple);
 
    heap_close(pg_index, RowExclusiveLock);
 }