From decbb28bc471cee9fcc7702f972d213b3218e1cc Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Tue, 27 Jan 2026 12:41:05 +0800 Subject: [PATCH v2] tablecmds: fix bug where index rebuild loses replica identity on partitions ALTER TABLE ... ALTER COLUMN TYPE may require rebuilding dependent indexes. When the rebuilt index is marked as REPLICA IDENTITY on a partitioned table, tablecmds previously failed to restore replica identity on the affected partitions, leaving logical replication misconfigured. Fix this by tracking replica identity indexes using OIDs and by recursively collecting replica identity indexes on all partitions of a partitioned table. After index rebuilds complete, restore replica identity markings for each affected table. Add regression tests covering multi-level partition hierarchies, including partitions in different schemas, to verify that replica identity is preserved across index rebuilds. Author: Chao Li Reviewed-by: Discussion: https://postgr.es/m/DB533C25-C6BA-4C0F-8046-96168E9CDD72@gmail.com --- src/backend/commands/tablecmds.c | 118 +++++++++++++----- .../regress/expected/replica_identity.out | 67 ++++++++++ src/test/regress/sql/replica_identity.sql | 40 ++++++ 3 files changed, 196 insertions(+), 29 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f976c0e5c7e..521d4fd2c2d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -204,7 +204,10 @@ typedef struct AlteredTableInfo List *changedConstraintDefs; /* string definitions of same */ List *changedIndexOids; /* OIDs of indexes to rebuild */ List *changedIndexDefs; /* string definitions of same */ - char *replicaIdentityIndex; /* index to reset as REPLICA IDENTITY */ + List *replicaIdentityIndexOids; /* OIDs of index to reset as + * REPLICA IDENTITY */ + List *replicaIdentityTableOids; /* OIDs of tables to reset as + * REPLICA IDENTITY */ char *clusterOnIndex; /* index to use for CLUSTER */ List *changedStatisticsOids; /* OIDs of statistics to rebuild */ List *changedStatisticsDefs; /* string definitions of same */ @@ -647,9 +650,9 @@ static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno); static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, - Relation rel, AttrNumber attnum, const char *colName); -static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab); -static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); + Relation rel, AttrNumber attnum, const char *colName, LOCKMODE lockmode); +static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab, LOCKMODE lockmode); +static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab, LOCKMODE lockmode); static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode); @@ -8715,7 +8718,7 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName, * etc), and record enough information to let us recreate the objects * after rewrite. */ - RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName); + RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName, lockmode); } /* @@ -14868,7 +14871,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, * the info before executing ALTER TYPE, though, else the deparser will * get confused. */ - RememberAllDependentForRebuilding(tab, AT_AlterColumnType, rel, attnum, colName); + RememberAllDependentForRebuilding(tab, AT_AlterColumnType, rel, attnum, colName, lockmode); /* * Now scan for dependencies of this column on other things. The only @@ -15071,7 +15074,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, */ static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, - Relation rel, AttrNumber attnum, const char *colName) + Relation rel, AttrNumber attnum, const char *colName, LOCKMODE lockmode) { Relation depRel; ScanKeyData key[3]; @@ -15117,7 +15120,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, relKind == RELKIND_PARTITIONED_INDEX) { Assert(foundObject.objectSubId == 0); - RememberIndexForRebuilding(foundObject.objectId, tab); + RememberIndexForRebuilding(foundObject.objectId, tab, lockmode); } else if (relKind == RELKIND_SEQUENCE) { @@ -15138,7 +15141,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, case ConstraintRelationId: Assert(foundObject.objectSubId == 0); - RememberConstraintForRebuilding(foundObject.objectId, tab); + RememberConstraintForRebuilding(foundObject.objectId, tab, lockmode); break; case ProcedureRelationId: @@ -15291,20 +15294,65 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, table_close(depRel, NoLock); } +static void +find_partition_replica_identity_indexes(AlteredTableInfo *tab, Oid relId, Oid indexId, LOCKMODE lockmode) +{ + List *partRelIds = NIL; + + partRelIds = find_inheritance_children(relId, lockmode); + foreach_oid(partRelOid, partRelIds) + { + Relation partRel; + Oid partIndexId; + + /* find_inheritance_children already got lock */ + partRel = relation_open(partRelOid, lockmode); + + partIndexId = index_get_partition(partRel, indexId); + if (!OidIsValid(partIndexId)) + { + relation_close(partRel, NoLock); + continue; + } + + if (get_index_isreplident(partIndexId)) + { + tab->replicaIdentityIndexOids = lappend_oid(tab->replicaIdentityIndexOids, partIndexId); + tab->replicaIdentityTableOids = lappend_oid(tab->replicaIdentityTableOids, partRelOid); + } + + if (partRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + find_partition_replica_identity_indexes(tab, partRelOid, partIndexId, lockmode); + } + + relation_close(partRel, NoLock); + } + list_free(partRelIds); +} + /* * Subroutine for ATExecAlterColumnType: remember that a replica identity * needs to be reset. */ static void -RememberReplicaIdentityForRebuilding(Oid indoid, AlteredTableInfo *tab) +RememberReplicaIdentityForRebuilding(Oid indoid, AlteredTableInfo *tab, LOCKMODE lockmode) { if (!get_index_isreplident(indoid)) return; - if (tab->replicaIdentityIndex) + if (tab->replicaIdentityIndexOids != NIL) elog(ERROR, "relation %u has multiple indexes marked as replica identity", tab->relid); - tab->replicaIdentityIndex = get_rel_name(indoid); + tab->replicaIdentityIndexOids = lappend_oid(tab->replicaIdentityIndexOids, indoid); + tab->replicaIdentityTableOids = lappend_oid(tab->replicaIdentityTableOids, tab->relid); + + /* For regular tables, we can only have one replica identity index */ + if (tab->rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) + return; + + /* For partitioned tables, find all partitions' replica identity indexes */ + find_partition_replica_identity_indexes(tab, tab->relid, indoid, lockmode); } /* @@ -15327,7 +15375,7 @@ RememberClusterOnForRebuilding(Oid indoid, AlteredTableInfo *tab) * to be rebuilt (which we might already know). */ static void -RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab) +RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab, LOCKMODE lockmode) { /* * This de-duplication check is critical for two independent reasons: we @@ -15372,7 +15420,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab) indoid = get_constraint_index(conoid); if (OidIsValid(indoid)) { - RememberReplicaIdentityForRebuilding(indoid, tab); + RememberReplicaIdentityForRebuilding(indoid, tab, lockmode); RememberClusterOnForRebuilding(indoid, tab); } } @@ -15383,7 +15431,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab) * to be rebuilt (which we might already know). */ static void -RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) +RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab, LOCKMODE lockmode) { /* * This de-duplication check is critical for two independent reasons: we @@ -15406,7 +15454,7 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) if (OidIsValid(conoid)) { - RememberConstraintForRebuilding(conoid, tab); + RememberConstraintForRebuilding(conoid, tab, lockmode); } else { @@ -15423,7 +15471,7 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) * or if it is a clustered index, so that ATPostAlterTypeCleanup() * can queue up commands necessary to restore those properties. */ - RememberReplicaIdentityForRebuilding(indoid, tab); + RememberReplicaIdentityForRebuilding(indoid, tab, lockmode); RememberClusterOnForRebuilding(indoid, tab); } } @@ -15602,19 +15650,31 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) /* * Queue up command to restore replica identity index marking */ - if (tab->replicaIdentityIndex) + if (tab->replicaIdentityIndexOids != NIL) { - AlterTableCmd *cmd = makeNode(AlterTableCmd); - ReplicaIdentityStmt *subcmd = makeNode(ReplicaIdentityStmt); - - subcmd->identity_type = REPLICA_IDENTITY_INDEX; - subcmd->name = tab->replicaIdentityIndex; - cmd->subtype = AT_ReplicaIdentity; - cmd->def = (Node *) subcmd; - - /* do it after indexes and constraints */ - tab->subcmds[AT_PASS_OLD_CONSTR] = - lappend(tab->subcmds[AT_PASS_OLD_CONSTR], cmd); + forboth(oid_item, tab->replicaIdentityIndexOids, + def_item, tab->replicaIdentityTableOids) + { + Oid indexId = lfirst_oid(oid_item); + Oid relId = lfirst_oid(def_item); + Relation partrel; + AlteredTableInfo *parttab; + + AlterTableCmd *cmd = makeNode(AlterTableCmd); + ReplicaIdentityStmt *subcmd = makeNode(ReplicaIdentityStmt); + + subcmd->identity_type = REPLICA_IDENTITY_INDEX; + subcmd->name = get_rel_name(indexId); + cmd->subtype = AT_ReplicaIdentity; + cmd->def = (Node *) subcmd; + + /* do it after indexes and constraints */ + partrel = relation_open(relId, lockmode); + parttab = ATGetQueueEntry(wqueue, partrel); + parttab->subcmds[AT_PASS_OLD_CONSTR] = + lappend(parttab->subcmds[AT_PASS_OLD_CONSTR], cmd); + relation_close(partrel, NoLock); + } } /* diff --git a/src/test/regress/expected/replica_identity.out b/src/test/regress/expected/replica_identity.out index b9b8dde018f..0cdf7666fb9 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -290,6 +290,73 @@ ALTER TABLE test_replica_identity5 DROP CONSTRAINT test_replica_identity5_pkey; ERROR: constraint "test_replica_identity5_pkey" of relation "test_replica_identity5" does not exist ALTER TABLE test_replica_identity5 ALTER b DROP NOT NULL; ERROR: column "b" is in index used as replica identity +-- +-- Test index rebuild preserves replica identity against partitioned tables +-- +CREATE TABLE test_replica_identity_partitioned (id int NOT NULL, val int NOT NULL) PARTITION BY RANGE (id); +CREATE TABLE test_replica_identity_partitioned_p1 PARTITION OF test_replica_identity_partitioned + FOR VALUES FROM (0) TO (100); +CREATE TABLE test_replica_identity_partitioned_p2 PARTITION OF test_replica_identity_partitioned + FOR VALUES FROM (101) TO (200) PARTITION BY LIST (id); +CREATE TABLE test_replica_identity_partitioned_p2_1 PARTITION OF test_replica_identity_partitioned_p2 + FOR VALUES IN (150, 160); +-- For better coverage, create a parition in a different schema +CREATE SCHEMA test_replica_identity_schema; +CREATE TABLE test_replica_identity_schema.test_replica_identity_partitioned_p2_2 PARTITION OF test_replica_identity_partitioned_p2 + FOR VALUES IN (170, 180); +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id, val); +SELECT tc.relname as table_name, tc.relreplident, i.indexrelid::regclass AS index_name, i.indisreplident FROM pg_class c + LEFT JOIN pg_index i ON c.oid = i.indexrelid + LEFT JOIN pg_class tc ON i.indrelid = tc.oid + WHERE c.relname LIKE 'test_replica_identity_partitioned%' and c.relkind in ('i', 'I') + ORDER BY c.relname; + table_name | relreplident | index_name | indisreplident +----------------------------------------+--------------+--------------------------------------------------------------------------------+---------------- + test_replica_identity_partitioned_p1 | d | test_replica_identity_partitioned_p1_id_val_idx | f + test_replica_identity_partitioned_p2_1 | d | test_replica_identity_partitioned_p2_1_id_val_idx | f + test_replica_identity_partitioned_p2_2 | d | test_replica_identity_schema.test_replica_identity_partitioned_p2_2_id_val_idx | f + test_replica_identity_partitioned_p2 | d | test_replica_identity_partitioned_p2_id_val_idx | f + test_replica_identity_partitioned | d | test_replica_identity_partitioned_pkey | f +(5 rows) + +ALTER TABLE test_replica_identity_partitioned REPLICA IDENTITY USING INDEX test_replica_identity_partitioned_pkey; +ALTER TABLE test_replica_identity_partitioned_p1 REPLICA IDENTITY USING INDEX test_replica_identity_partitioned_p1_id_val_idx; +ALTER TABLE test_replica_identity_partitioned_p2_1 REPLICA IDENTITY USING INDEX test_replica_identity_partitioned_p2_1_id_val_idx; +ALTER TABLE test_replica_identity_schema.test_replica_identity_partitioned_p2_2 REPLICA IDENTITY USING INDEX test_replica_identity_partitioned_p2_2_id_val_idx; +SELECT tc.relname as table_name, tc.relreplident, i.indexrelid::regclass AS index_name, i.indisreplident FROM pg_class c + LEFT JOIN pg_index i ON c.oid = i.indexrelid + LEFT JOIN pg_class tc ON i.indrelid = tc.oid + WHERE c.relname LIKE 'test_replica_identity_partitioned%' and c.relkind in ('i', 'I') + ORDER BY c.relname; + table_name | relreplident | index_name | indisreplident +----------------------------------------+--------------+--------------------------------------------------------------------------------+---------------- + test_replica_identity_partitioned_p1 | i | test_replica_identity_partitioned_p1_id_val_idx | t + test_replica_identity_partitioned_p2_1 | i | test_replica_identity_partitioned_p2_1_id_val_idx | t + test_replica_identity_partitioned_p2_2 | i | test_replica_identity_schema.test_replica_identity_partitioned_p2_2_id_val_idx | t + test_replica_identity_partitioned_p2 | d | test_replica_identity_partitioned_p2_id_val_idx | f + test_replica_identity_partitioned | i | test_replica_identity_partitioned_pkey | t +(5 rows) + +ALTER TABLE test_replica_identity_partitioned ALTER COLUMN val TYPE bigint; +SELECT tc.relname as table_name, tc.relreplident, i.indexrelid::regclass AS index_name, i.indisreplident FROM pg_class c + LEFT JOIN pg_index i ON c.oid = i.indexrelid + LEFT JOIN pg_class tc ON i.indrelid = tc.oid + WHERE c.relname LIKE 'test_replica_identity_partitioned%' and c.relkind in ('i', 'I') + ORDER BY c.relname; + table_name | relreplident | index_name | indisreplident +----------------------------------------+--------------+--------------------------------------------------------------------------------+---------------- + test_replica_identity_partitioned_p1 | i | test_replica_identity_partitioned_p1_id_val_idx | t + test_replica_identity_partitioned_p2_1 | i | test_replica_identity_partitioned_p2_1_id_val_idx | t + test_replica_identity_partitioned_p2_2 | i | test_replica_identity_schema.test_replica_identity_partitioned_p2_2_id_val_idx | t + test_replica_identity_partitioned_p2 | d | test_replica_identity_partitioned_p2_id_val_idx | f + test_replica_identity_partitioned | i | test_replica_identity_partitioned_pkey | t +(5 rows) + +DROP SCHEMA test_replica_identity_schema CASCADE; +NOTICE: drop cascades to table test_replica_identity_schema.test_replica_identity_partitioned_p2_2 +-- +-- Cleanup +-- DROP TABLE test_replica_identity; DROP TABLE test_replica_identity2; DROP TABLE test_replica_identity3; diff --git a/src/test/regress/sql/replica_identity.sql b/src/test/regress/sql/replica_identity.sql index 30daec05b71..20e62b2e4f2 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -134,6 +134,46 @@ ALTER TABLE test_replica_identity5 ALTER b SET NOT NULL; ALTER TABLE test_replica_identity5 DROP CONSTRAINT test_replica_identity5_pkey; ALTER TABLE test_replica_identity5 ALTER b DROP NOT NULL; +-- +-- Test index rebuild preserves replica identity against partitioned tables +-- +CREATE TABLE test_replica_identity_partitioned (id int NOT NULL, val int NOT NULL) PARTITION BY RANGE (id); +CREATE TABLE test_replica_identity_partitioned_p1 PARTITION OF test_replica_identity_partitioned + FOR VALUES FROM (0) TO (100); +CREATE TABLE test_replica_identity_partitioned_p2 PARTITION OF test_replica_identity_partitioned + FOR VALUES FROM (101) TO (200) PARTITION BY LIST (id); +CREATE TABLE test_replica_identity_partitioned_p2_1 PARTITION OF test_replica_identity_partitioned_p2 + FOR VALUES IN (150, 160); +-- For better coverage, create a parition in a different schema +CREATE SCHEMA test_replica_identity_schema; +CREATE TABLE test_replica_identity_schema.test_replica_identity_partitioned_p2_2 PARTITION OF test_replica_identity_partitioned_p2 + FOR VALUES IN (170, 180); +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id, val); +SELECT tc.relname as table_name, tc.relreplident, i.indexrelid::regclass AS index_name, i.indisreplident FROM pg_class c + LEFT JOIN pg_index i ON c.oid = i.indexrelid + LEFT JOIN pg_class tc ON i.indrelid = tc.oid + WHERE c.relname LIKE 'test_replica_identity_partitioned%' and c.relkind in ('i', 'I') + ORDER BY c.relname; +ALTER TABLE test_replica_identity_partitioned REPLICA IDENTITY USING INDEX test_replica_identity_partitioned_pkey; +ALTER TABLE test_replica_identity_partitioned_p1 REPLICA IDENTITY USING INDEX test_replica_identity_partitioned_p1_id_val_idx; +ALTER TABLE test_replica_identity_partitioned_p2_1 REPLICA IDENTITY USING INDEX test_replica_identity_partitioned_p2_1_id_val_idx; +ALTER TABLE test_replica_identity_schema.test_replica_identity_partitioned_p2_2 REPLICA IDENTITY USING INDEX test_replica_identity_partitioned_p2_2_id_val_idx; +SELECT tc.relname as table_name, tc.relreplident, i.indexrelid::regclass AS index_name, i.indisreplident FROM pg_class c + LEFT JOIN pg_index i ON c.oid = i.indexrelid + LEFT JOIN pg_class tc ON i.indrelid = tc.oid + WHERE c.relname LIKE 'test_replica_identity_partitioned%' and c.relkind in ('i', 'I') + ORDER BY c.relname; +ALTER TABLE test_replica_identity_partitioned ALTER COLUMN val TYPE bigint; +SELECT tc.relname as table_name, tc.relreplident, i.indexrelid::regclass AS index_name, i.indisreplident FROM pg_class c + LEFT JOIN pg_index i ON c.oid = i.indexrelid + LEFT JOIN pg_class tc ON i.indrelid = tc.oid + WHERE c.relname LIKE 'test_replica_identity_partitioned%' and c.relkind in ('i', 'I') + ORDER BY c.relname; +DROP SCHEMA test_replica_identity_schema CASCADE; + +-- +-- Cleanup +-- DROP TABLE test_replica_identity; DROP TABLE test_replica_identity2; DROP TABLE test_replica_identity3; -- 2.50.1 (Apple Git-155)