From: Michael Paquier Date: Mon, 27 Jul 2026 23:33:23 +0000 (+0900) Subject: Fix propagation of indimmediate flag in index_create_copy() X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=74276e685dd01a8834f05f7d8b29a140a264e127;p=thirdparty%2Fpostgresql.git Fix propagation of indimmediate flag in index_create_copy() index_create_copy is used to create copy definitions of existing indexes. Currently, it passes 0 as constr_flags to index_create(), which results in the copied index to always be created as immediate (indimmediate set to true). For deferrable unique constraints, it means that the transient index used during the phase 2 of REINDEX CONCURRENTLY forces immediate constraint checks on concurrent inserts, which can cause unexpected constraint violations based on the definition of the parent table, inconsistently set in the copied index. To fix this without violating the contract of constr_flags (which should only be used when creating constraints) and without relaxing the strict assertion in index_create(), this introduces a new index creation flag: INDEX_CREATE_DEFERRABLE. If set, a copied index's indimmediate is set to false, meaning that unique constraints are not enforced immediately on insertion, but at transaction commit time. An isolation test for REINDEX CONCURRENTLY is added, based on an injection point waiting after phase 1 of the operation, where an index copy has been built and is able to accept DMLs for its validation in phase 2. The test is tentatively backpatched down to v17. INJECTION_POINT() is outside a transaction context, which should be fine on HEAD since 8daeaa9b642c but I suspect may cause issues in v19 and older branches due to the wait facility depending on condition variables and a DSM setup, but let's see what the buildfarm tells. Author: Nitin Motiani Discussion: https://postgr.es/m/CAH5HC97JmjPpgiQOqW9xm8qXhNiu7zZ1Qh+FfhEESJuDv69kuQ@mail.gmail.com Backpatch-through: 14 --- diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 31ef84d0a16..7c1b94407a9 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -717,6 +717,9 @@ UpdateIndexRelation(Oid indexoid, * create a partitioned index (table must be partitioned) * INDEX_CREATE_SUPPRESS_PROGRESS: * don't report progress during the index build. + * INDEX_CREATE_DEFERRABLE: + * index supports a deferrable constraint, mark it as + * non-immediate (indimmediate = false). * * constr_flags: flags passed to index_constraint_create * (only if INDEX_CREATE_ADD_CONSTRAINT is set) @@ -1051,7 +1054,8 @@ index_create(Relation heapRelation, indexInfo, collationIds, opclassIds, coloptions, isprimary, is_exclusion, - (constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) == 0, + (constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) == 0 && + (flags & INDEX_CREATE_DEFERRABLE) == 0, !concurrent && !invalid, !concurrent); @@ -1324,6 +1328,7 @@ index_create_copy(Relation heapRelation, uint16 flags, List *indexColNames = NIL; List *indexExprs = NIL; List *indexPreds = NIL; + Form_pg_index indexForm; indexRelation = index_open(oldIndexId, RowExclusiveLock); @@ -1343,6 +1348,13 @@ index_create_copy(Relation heapRelation, uint16 flags, indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(oldIndexId)); if (!HeapTupleIsValid(indexTuple)) elog(ERROR, "cache lookup failed for index %u", oldIndexId); + + indexForm = (Form_pg_index) GETSTRUCT(indexTuple); + + /* Old index is deferrable, do the same for the new index */ + if (!indexForm->indimmediate) + flags |= INDEX_CREATE_DEFERRABLE; + indclassDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple, Anum_pg_index_indclass); indclass = (oidvector *) DatumGetPointer(indclassDatum); @@ -1477,7 +1489,7 @@ index_create_copy(Relation heapRelation, uint16 flags, stattargets, reloptionsDatum, flags, - 0, + 0, /* constr_flags */ true, /* allow table to be a system catalog? */ false, /* is_internal? */ NULL); diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index b71e588a953..3790b8e1252 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -4296,6 +4296,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein CommitTransactionCommand(); } + INJECTION_POINT("reindex-conc-index-built", NULL); + StartTransactionCommand(); /* diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index 9aee8226347..b952ad071d3 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -72,6 +72,7 @@ extern void index_check_primary_key(Relation heapRel, #define INDEX_CREATE_PARTITIONED (1 << 5) #define INDEX_CREATE_INVALID (1 << 6) #define INDEX_CREATE_SUPPRESS_PROGRESS (1 << 7) +#define INDEX_CREATE_DEFERRABLE (1 << 8) extern Oid index_create(Relation heapRelation, const char *indexRelationName, diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index fac80f3a4a7..25a3ddd890d 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -14,6 +14,7 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress ISOLATION = basic \ inplace \ + reindex_concurrently_deferred \ repack \ repack_temporal \ repack_temporal_multirange \ diff --git a/src/test/modules/injection_points/expected/reindex_concurrently_deferred.out b/src/test/modules/injection_points/expected/reindex_concurrently_deferred.out new file mode 100644 index 00000000000..39924fa24fe --- /dev/null +++ b/src/test/modules/injection_points/expected/reindex_concurrently_deferred.out @@ -0,0 +1,41 @@ +Parsed test spec with 2 sessions + +starting permutation: reindex check_catalog begin2 write2 write_dup resolve_dup commit2 wakeup noop1 +injection_points_attach +----------------------- + +(1 row) + +step reindex: REINDEX TABLE CONCURRENTLY reind_deferred; +step check_catalog: + SELECT c.relname, i.indisunique, i.indimmediate, i.indisready, i.indisvalid + FROM pg_class c + JOIN pg_index i ON i.indexrelid = c.oid + WHERE c.relname = 'uq_val_ccnew'; + +relname |indisunique|indimmediate|indisready|indisvalid +------------+-----------+------------+----------+---------- +uq_val_ccnew|t |f |t |f +(1 row) + +step begin2: BEGIN; +step write2: INSERT INTO reind_deferred VALUES (3, 9); +step write_dup: INSERT INTO reind_deferred VALUES (4, 9); +step resolve_dup: UPDATE reind_deferred SET val = 10 WHERE id = 4; +step commit2: COMMIT; +step wakeup: + SELECT injection_points_detach('reindex-conc-index-built'); + SELECT injection_points_wakeup('reindex-conc-index-built'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step reindex: <... completed> +step noop1: diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 163b6374ebc..aaf0536ba7e 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -45,6 +45,7 @@ tests += { 'specs': [ 'basic', 'inplace', + 'reindex_concurrently_deferred', 'repack', 'repack_temporal', 'repack_temporal_multirange', diff --git a/src/test/modules/injection_points/specs/reindex_concurrently_deferred.spec b/src/test/modules/injection_points/specs/reindex_concurrently_deferred.spec new file mode 100644 index 00000000000..4b95e1da2a7 --- /dev/null +++ b/src/test/modules/injection_points/specs/reindex_concurrently_deferred.spec @@ -0,0 +1,50 @@ +# REINDEX CONCURRENTLY with DEFERRED constraints +# +# Verify that concurrent writes that temporarily violate a deferred unique +# constraint do not fail while REINDEX CONCURRENTLY is running. +# +# The injection point "reindex-conc-index-built" fires after the phase 2 +# of REINDEX CONCURRENTLY, when the new index has indisready = true (inserts +# are checked against it) but indisvalid = false. + +setup +{ + CREATE EXTENSION injection_points; + CREATE TABLE reind_deferred (id int, val int, + CONSTRAINT uq_val UNIQUE(val) DEFERRABLE INITIALLY DEFERRED); + INSERT INTO reind_deferred VALUES (1, 1), (2, 2); +} + +teardown +{ + DROP TABLE reind_deferred; + DROP EXTENSION injection_points; +} + +session s1 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('reindex-conc-index-built', 'wait'); +} +step reindex { REINDEX TABLE CONCURRENTLY reind_deferred; } +step noop1 { } + +session s2 +step check_catalog { + SELECT c.relname, i.indisunique, i.indimmediate, i.indisready, i.indisvalid + FROM pg_class c + JOIN pg_index i ON i.indexrelid = c.oid + WHERE c.relname = 'uq_val_ccnew'; +} +step begin2 { BEGIN; } +step write2 { INSERT INTO reind_deferred VALUES (3, 9); } +step write_dup { INSERT INTO reind_deferred VALUES (4, 9); } +step resolve_dup { UPDATE reind_deferred SET val = 10 WHERE id = 4; } +step commit2 { COMMIT; } +step wakeup { + SELECT injection_points_detach('reindex-conc-index-built'); + SELECT injection_points_wakeup('reindex-conc-index-built'); +} + +permutation reindex check_catalog begin2 write2 write_dup resolve_dup commit2 wakeup noop1