diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 64a45f4ed4..47c84f4a07 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1568,7 +1568,6 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName) /* Preserve indisreplident in the new index */ newIndexForm->indisreplident = oldIndexForm->indisreplident; - oldIndexForm->indisreplident = false; /* Preserve indisclustered in the new index */ newIndexForm->indisclustered = oldIndexForm->indisclustered; @@ -1580,6 +1579,7 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName) newIndexForm->indisvalid = true; oldIndexForm->indisvalid = false; oldIndexForm->indisclustered = false; + oldIndexForm->indisreplident = false; CatalogTupleUpdate(pg_index, &oldIndexTuple->t_self, oldIndexTuple); CatalogTupleUpdate(pg_index, &newIndexTuple->t_self, newIndexTuple); @@ -3463,10 +3463,12 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action) * CONCURRENTLY that failed partway through.) * * Note: the CLUSTER logic assumes that indisclustered cannot be - * set on any invalid index, so clear that flag too. + * set on any invalid index, so clear that flag too. For + * cleanliness, also clear indisreplident. */ indexForm->indisvalid = false; indexForm->indisclustered = false; + indexForm->indisreplident = false; break; case INDEX_DROP_SET_DEAD: @@ -3478,6 +3480,8 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action) * the index at all. */ Assert(!indexForm->indisvalid); + Assert(!indexForm->indisclustered); + Assert(!indexForm->indisreplident); indexForm->indisready = false; indexForm->indislive = false; break; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index bcc7d3c29c..5d0dcfa3e6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -14892,7 +14892,10 @@ ATExecDropOf(Relation rel, LOCKMODE lockmode) * relation_mark_replica_identity: Update a table's replica identity * * Iff ri_type = REPLICA_IDENTITY_INDEX, indexOid must be the Oid of a suitable - * index. Otherwise, it should be InvalidOid. + * index. Otherwise, it must be InvalidOid. + * + * Caller had better hold an exclusive lock on the relation, as the results + * of running two of these concurrently wouldn't be pretty. */ static void relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid, @@ -14904,7 +14907,6 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid, HeapTuple pg_index_tuple; Form_pg_class pg_class_form; Form_pg_index pg_index_form; - ListCell *index; /* @@ -14926,29 +14928,7 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid, heap_freetuple(pg_class_tuple); /* - * Check whether the correct index is marked indisreplident; if so, we're - * done. - */ - if (OidIsValid(indexOid)) - { - Assert(ri_type == REPLICA_IDENTITY_INDEX); - - pg_index_tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid)); - if (!HeapTupleIsValid(pg_index_tuple)) - elog(ERROR, "cache lookup failed for index %u", indexOid); - pg_index_form = (Form_pg_index) GETSTRUCT(pg_index_tuple); - - if (pg_index_form->indisreplident) - { - ReleaseSysCache(pg_index_tuple); - return; - } - ReleaseSysCache(pg_index_tuple); - } - - /* - * Clear the indisreplident flag from any index that had it previously, - * and set it for any index that should have it now. + * Update the per-index indisreplident flags correctly. */ pg_index = table_open(IndexRelationId, RowExclusiveLock); foreach(index, RelationGetIndexList(rel)) @@ -14962,19 +14942,23 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid, elog(ERROR, "cache lookup failed for index %u", thisIndexOid); pg_index_form = (Form_pg_index) GETSTRUCT(pg_index_tuple); - /* - * Unset the bit if set. We know it's wrong because we checked this - * earlier. - */ - if (pg_index_form->indisreplident) + if (thisIndexOid == indexOid) { - dirty = true; - pg_index_form->indisreplident = false; + /* Set the bit if not already set. */ + if (!pg_index_form->indisreplident) + { + dirty = true; + pg_index_form->indisreplident = true; + } } - else if (thisIndexOid == indexOid) + else { - dirty = true; - pg_index_form->indisreplident = true; + /* Unset the bit if set. */ + if (pg_index_form->indisreplident) + { + dirty = true; + pg_index_form->indisreplident = false; + } } if (dirty) @@ -14985,7 +14969,9 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid, /* * Invalidate the relcache for the table, so that after we commit * all sessions will refresh the table's replica identity index - * before attempting any UPDATE or DELETE on the table. + * before attempting any UPDATE or DELETE on the table. (If we + * changed the table's pg_class row above, then a relcache inval + * is already queued due to that; but we might not have.) */ CacheInvalidateRelcache(rel); } @@ -15071,12 +15057,6 @@ ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot use partial index \"%s\" as replica identity", RelationGetRelationName(indexRel)))); - /* And neither are invalid indexes. */ - if (!indexRel->rd_index->indisvalid) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot use invalid index \"%s\" as replica identity", - RelationGetRelationName(indexRel)))); /* Check index for nullable columns. */ for (key = 0; key < IndexRelationGetNumberOfKeyAttributes(indexRel); key++) diff --git a/src/test/regress/expected/replica_identity.out b/src/test/regress/expected/replica_identity.out index e25ec06a84..7d798ef2a5 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -227,7 +227,44 @@ Indexes: -- used as replica identity. ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL; ERROR: column "id" is in index used as replica identity +-- +-- Test that replica identity can be set on an index that's not yet valid. +-- (This matches the way pg_dump will try to dump a partitioned table.) +-- +CREATE TABLE test_replica_identity4(id integer NOT NULL) PARTITION BY LIST (id); +CREATE TABLE test_replica_identity4_1(id integer NOT NULL); +ALTER TABLE ONLY test_replica_identity4 + ATTACH PARTITION test_replica_identity4_1 FOR VALUES IN (1); +ALTER TABLE ONLY test_replica_identity4 + ADD CONSTRAINT test_replica_identity4_pkey PRIMARY KEY (id); +ALTER TABLE ONLY test_replica_identity4 + REPLICA IDENTITY USING INDEX test_replica_identity4_pkey; +ALTER TABLE ONLY test_replica_identity4_1 + ADD CONSTRAINT test_replica_identity4_1_pkey PRIMARY KEY (id); +\d+ test_replica_identity4 + Partitioned table "public.test_replica_identity4" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+------------- + id | integer | | not null | | plain | | +Partition key: LIST (id) +Indexes: + "test_replica_identity4_pkey" PRIMARY KEY, btree (id) INVALID REPLICA IDENTITY +Partitions: test_replica_identity4_1 FOR VALUES IN (1) + +ALTER INDEX test_replica_identity4_pkey + ATTACH PARTITION test_replica_identity4_1_pkey; +\d+ test_replica_identity4 + Partitioned table "public.test_replica_identity4" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+------------- + id | integer | | not null | | plain | | +Partition key: LIST (id) +Indexes: + "test_replica_identity4_pkey" PRIMARY KEY, btree (id) REPLICA IDENTITY +Partitions: test_replica_identity4_1 FOR VALUES IN (1) + DROP TABLE test_replica_identity; DROP TABLE test_replica_identity2; DROP TABLE test_replica_identity3; +DROP TABLE test_replica_identity4; DROP TABLE test_replica_identity_othertable; diff --git a/src/test/regress/sql/replica_identity.sql b/src/test/regress/sql/replica_identity.sql index 33da829713..14620b7713 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -98,7 +98,27 @@ ALTER TABLE test_replica_identity3 ALTER COLUMN id TYPE bigint; -- used as replica identity. ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL; +-- +-- Test that replica identity can be set on an index that's not yet valid. +-- (This matches the way pg_dump will try to dump a partitioned table.) +-- +CREATE TABLE test_replica_identity4(id integer NOT NULL) PARTITION BY LIST (id); +CREATE TABLE test_replica_identity4_1(id integer NOT NULL); +ALTER TABLE ONLY test_replica_identity4 + ATTACH PARTITION test_replica_identity4_1 FOR VALUES IN (1); +ALTER TABLE ONLY test_replica_identity4 + ADD CONSTRAINT test_replica_identity4_pkey PRIMARY KEY (id); +ALTER TABLE ONLY test_replica_identity4 + REPLICA IDENTITY USING INDEX test_replica_identity4_pkey; +ALTER TABLE ONLY test_replica_identity4_1 + ADD CONSTRAINT test_replica_identity4_1_pkey PRIMARY KEY (id); +\d+ test_replica_identity4 +ALTER INDEX test_replica_identity4_pkey + ATTACH PARTITION test_replica_identity4_1_pkey; +\d+ test_replica_identity4 + DROP TABLE test_replica_identity; DROP TABLE test_replica_identity2; DROP TABLE test_replica_identity3; +DROP TABLE test_replica_identity4; DROP TABLE test_replica_identity_othertable;