public inbox for [email protected]help / color / mirror / Atom feed
Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 9+ messages / 2 participants [nested] [flat]
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions @ 2026-01-27 08:30 Chao Li <[email protected]> 2026-01-28 02:49 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Chao Li @ 2026-01-27 08:30 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Postgres hackers <[email protected]> > On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: > > > >> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: >> >> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: >>> I found this bug while working on a related patch [1]. >>> >>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and >>> that index is used as REPLICA IDENTITY on a partitioned table, the >>> replica identity marking on partitions can be silently lost after the >>> rebuild. >> >> I am slightly confused by the tests included in the proposed patch. >> On HEAD, if I undo the proposed changes of tablecmds.c, the tests >> pass. If I run the tests of the patch with the changes of >> tablecmds.c, the tests also pass. > > Oops, that isn’t supposed to be so. I’ll check the test. > Okay, I see the problem is here: ``` +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); ``` I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? Please see v2, the test should fail on master now. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v2-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch (20.0K, 2-v2-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch) download | inline diff: From decbb28bc471cee9fcc7702f972d213b3218e1cc Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> 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 <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/[email protected] --- 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) ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> @ 2026-01-28 02:49 ` Chao Li <[email protected]> 2026-02-26 06:59 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Chao Li @ 2026-01-28 02:49 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Postgres hackers <[email protected]> > On Jan 27, 2026, at 16:30, Chao Li <[email protected]> wrote: > > > >> On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: >> >> >> >>> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: >>> >>> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: >>>> I found this bug while working on a related patch [1]. >>>> >>>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and >>>> that index is used as REPLICA IDENTITY on a partitioned table, the >>>> replica identity marking on partitions can be silently lost after the >>>> rebuild. >>> >>> I am slightly confused by the tests included in the proposed patch. >>> On HEAD, if I undo the proposed changes of tablecmds.c, the tests >>> pass. If I run the tests of the patch with the changes of >>> tablecmds.c, the tests also pass. >> >> Oops, that isn’t supposed to be so. I’ll check the test. >> > > Okay, I see the problem is here: > ``` > +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); > ``` > > I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. > > Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? I just updated the test to store index OIDs before and after rebuild into 2 temp tables, so that we can compare the OIDs to verify rebuild happens and replica identity preserved. I tried to port the test to master branch, and the test failed. From the test diff file, we can see replica identity lost on 3 leaf partitions: ``` @@ -360,9 +360,9 @@ ORDER BY b.index_name; index_name | rebuilt | ri_lost ---------------------------------------------------+---------+--------- - test_replica_identity_partitioned_p1_id_val_idx | t | f - test_replica_identity_partitioned_p2_1_id_val_idx | t | f - test_replica_identity_partitioned_p2_2_id_val_idx | t | f + test_replica_identity_partitioned_p1_id_val_idx | t | t + test_replica_identity_partitioned_p2_1_id_val_idx | t | t + test_replica_identity_partitioned_p2_2_id_val_idx | t | t test_replica_identity_partitioned_p2_id_val_idx | t | f test_replica_identity_partitioned_pkey | t | f (5 rows) ``` With this patch, the test passes and all replica identity are preserved. PFA v3: * Enhanced the test. * A small change in find_partition_replica_identity_indexes(): if we will not update a partition, then unlock it. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v3-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch (21.4K, 2-v3-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch) download | inline diff: From 018fffd8eccbd99927e7c9e7e829c7be1a5c35d5 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Tue, 27 Jan 2026 12:41:05 +0800 Subject: [PATCH v3] 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 <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/tablecmds.c | 119 +++++++++++++----- .../regress/expected/replica_identity.out | 82 ++++++++++++ src/test/regress/sql/replica_identity.sql | 55 ++++++++ 3 files changed, 227 insertions(+), 29 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f976c0e5c7e..2d3142e6ff8 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,66 @@ 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)) + { + /* if we won't touch the partion, then unlock it */ + relation_close(partRel, lockmode); + 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 +15376,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 +15421,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 +15432,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 +15455,7 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) if (OidIsValid(conoid)) { - RememberConstraintForRebuilding(conoid, tab); + RememberConstraintForRebuilding(conoid, tab, lockmode); } else { @@ -15423,7 +15472,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 +15651,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..8ff9da55569 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -290,6 +290,88 @@ 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); +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) + +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; + 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) + +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; + index_name | rebuilt | ri_lost +---------------------------------------------------+---------+--------- + test_replica_identity_partitioned_p1_id_val_idx | t | f + test_replica_identity_partitioned_p2_1_id_val_idx | t | f + test_replica_identity_partitioned_p2_2_id_val_idx | t | f + test_replica_identity_partitioned_p2_id_val_idx | t | f + test_replica_identity_partitioned_pkey | t | f +(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..e9a71eac8a9 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -134,6 +134,61 @@ 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); +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; +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; +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) ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-01-28 02:49 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> @ 2026-02-26 06:59 ` Chao Li <[email protected]> 2026-03-21 10:29 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Chao Li @ 2026-02-26 06:59 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Postgres hackers <[email protected]> > On Jan 28, 2026, at 10:49, Chao Li <[email protected]> wrote: > > > >> On Jan 27, 2026, at 16:30, Chao Li <[email protected]> wrote: >> >> >> >>> On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: >>> >>> >>> >>>> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: >>>> >>>> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: >>>>> I found this bug while working on a related patch [1]. >>>>> >>>>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and >>>>> that index is used as REPLICA IDENTITY on a partitioned table, the >>>>> replica identity marking on partitions can be silently lost after the >>>>> rebuild. >>>> >>>> I am slightly confused by the tests included in the proposed patch. >>>> On HEAD, if I undo the proposed changes of tablecmds.c, the tests >>>> pass. If I run the tests of the patch with the changes of >>>> tablecmds.c, the tests also pass. >>> >>> Oops, that isn’t supposed to be so. I’ll check the test. >>> >> >> Okay, I see the problem is here: >> ``` >> +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); >> ``` >> >> I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. >> >> Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? > > I just updated the test to store index OIDs before and after rebuild into 2 temp tables, so that we can compare the OIDs to verify rebuild happens and replica identity preserved. > > I tried to port the test to master branch, and the test failed. From the test diff file, we can see replica identity lost on 3 leaf partitions: > ``` > @@ -360,9 +360,9 @@ > ORDER BY b.index_name; > index_name | rebuilt | ri_lost > ---------------------------------------------------+---------+--------- > - test_replica_identity_partitioned_p1_id_val_idx | t | f > - test_replica_identity_partitioned_p2_1_id_val_idx | t | f > - test_replica_identity_partitioned_p2_2_id_val_idx | t | f > + test_replica_identity_partitioned_p1_id_val_idx | t | t > + test_replica_identity_partitioned_p2_1_id_val_idx | t | t > + test_replica_identity_partitioned_p2_2_id_val_idx | t | t > test_replica_identity_partitioned_p2_id_val_idx | t | f > test_replica_identity_partitioned_pkey | t | f > (5 rows) > ``` > > With this patch, the test passes and all replica identity are preserved. > > PFA v3: > * Enhanced the test. > * A small change in find_partition_replica_identity_indexes(): if we will not update a partition, then unlock it. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > > > <v3-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch> The CF asked for a rebase, thus rebased as v4. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v4-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch (21.5K, 2-v4-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch) download | inline diff: From 4095569578df59cbb4ebc126c40c678743a8d386 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Tue, 27 Jan 2026 12:41:05 +0800 Subject: [PATCH v4] 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 <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/tablecmds.c | 119 +++++++++++++----- .../regress/expected/replica_identity.out | 82 ++++++++++++ src/test/regress/sql/replica_identity.sql | 55 ++++++++ 3 files changed, 227 insertions(+), 29 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b04b0dbd2a0..65d3eab7692 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); @@ -8713,7 +8716,7 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName, * Find everything that depends on the column (constraints, indexes, etc), * and record enough information to let us recreate the objects. */ - RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName); + RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName, lockmode); /* * Drop the dependency records of the GENERATED expression, in particular @@ -14865,7 +14868,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 @@ -15068,7 +15071,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]; @@ -15114,7 +15117,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) { @@ -15135,7 +15138,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, case ConstraintRelationId: Assert(foundObject.objectSubId == 0); - RememberConstraintForRebuilding(foundObject.objectId, tab); + RememberConstraintForRebuilding(foundObject.objectId, tab, lockmode); break; case ProcedureRelationId: @@ -15288,20 +15291,66 @@ 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)) + { + /* if we won't touch the partion, then unlock it */ + relation_close(partRel, lockmode); + 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); } /* @@ -15324,7 +15373,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 @@ -15369,7 +15418,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab) indoid = get_constraint_index(conoid); if (OidIsValid(indoid)) { - RememberReplicaIdentityForRebuilding(indoid, tab); + RememberReplicaIdentityForRebuilding(indoid, tab, lockmode); RememberClusterOnForRebuilding(indoid, tab); } } @@ -15380,7 +15429,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 @@ -15403,7 +15452,7 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) if (OidIsValid(conoid)) { - RememberConstraintForRebuilding(conoid, tab); + RememberConstraintForRebuilding(conoid, tab, lockmode); } else { @@ -15420,7 +15469,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); } } @@ -15599,19 +15648,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..8ff9da55569 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -290,6 +290,88 @@ 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); +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) + +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; + 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) + +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; + index_name | rebuilt | ri_lost +---------------------------------------------------+---------+--------- + test_replica_identity_partitioned_p1_id_val_idx | t | f + test_replica_identity_partitioned_p2_1_id_val_idx | t | f + test_replica_identity_partitioned_p2_2_id_val_idx | t | f + test_replica_identity_partitioned_p2_id_val_idx | t | f + test_replica_identity_partitioned_pkey | t | f +(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..e9a71eac8a9 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -134,6 +134,61 @@ 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); +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; +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; +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) ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-01-28 02:49 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-02-26 06:59 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> @ 2026-03-21 10:29 ` Xuneng Zhou <[email protected]> 2026-03-23 07:56 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Xuneng Zhou @ 2026-03-21 10:29 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Postgres hackers <[email protected]> On Thu, Mar 19, 2026 at 1:07 PM Chao Li <[email protected]> wrote: > > > > > On Feb 26, 2026, at 14:59, Chao Li <[email protected]> wrote: > > > > > > > >> On Jan 28, 2026, at 10:49, Chao Li <[email protected]> wrote: > >> > >> > >> > >>> On Jan 27, 2026, at 16:30, Chao Li <[email protected]> wrote: > >>> > >>> > >>> > >>>> On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: > >>>> > >>>> > >>>> > >>>>> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: > >>>>> > >>>>> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: > >>>>>> I found this bug while working on a related patch [1]. > >>>>>> > >>>>>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and > >>>>>> that index is used as REPLICA IDENTITY on a partitioned table, the > >>>>>> replica identity marking on partitions can be silently lost after the > >>>>>> rebuild. > >>>>> > >>>>> I am slightly confused by the tests included in the proposed patch. > >>>>> On HEAD, if I undo the proposed changes of tablecmds.c, the tests > >>>>> pass. If I run the tests of the patch with the changes of > >>>>> tablecmds.c, the tests also pass. > >>>> > >>>> Oops, that isn’t supposed to be so. I’ll check the test. > >>>> > >>> > >>> Okay, I see the problem is here: > >>> ``` > >>> +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); > >>> ``` > >>> > >>> I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. > >>> > >>> Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? > >> > >> I just updated the test to store index OIDs before and after rebuild into 2 temp tables, so that we can compare the OIDs to verify rebuild happens and replica identity preserved. > >> > >> I tried to port the test to master branch, and the test failed. From the test diff file, we can see replica identity lost on 3 leaf partitions: > >> ``` > >> @@ -360,9 +360,9 @@ > >> ORDER BY b.index_name; > >> index_name | rebuilt | ri_lost > >> ---------------------------------------------------+---------+--------- > >> - test_replica_identity_partitioned_p1_id_val_idx | t | f > >> - test_replica_identity_partitioned_p2_1_id_val_idx | t | f > >> - test_replica_identity_partitioned_p2_2_id_val_idx | t | f > >> + test_replica_identity_partitioned_p1_id_val_idx | t | t > >> + test_replica_identity_partitioned_p2_1_id_val_idx | t | t > >> + test_replica_identity_partitioned_p2_2_id_val_idx | t | t > >> test_replica_identity_partitioned_p2_id_val_idx | t | f > >> test_replica_identity_partitioned_pkey | t | f > >> (5 rows) > >> ``` > >> > >> With this patch, the test passes and all replica identity are preserved. > >> > >> PFA v3: > >> * Enhanced the test. > >> * A small change in find_partition_replica_identity_indexes(): if we will not update a partition, then unlock it. > >> > >> Best regards, > >> -- > >> Chao Li (Evan) > >> HighGo Software Co., Ltd. > >> https://www.highgo.com/ > >> > >> > >> > >> > >> <v3-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch> > > > > The CF asked for a rebase, thus rebased as v4. > > Hi, I reproduced this with the test case, and the patch appears to resolve it. Some comments on v5: -- Whether it makes sense to use a single list of pair structs instead of two parallel OID lists (replicaIdentityIndexOids + replicaIdentityTableOids) to avoid accidental desync. -- It would be better to make lock handling in find_partition_replica_identity_indexes() consistent (relation_open(..., NoLock) if child is already locked, and avoid mixed relation_close(..., lockmode)/NoLock behavior). -- Some typos in comments/tests (partion/parition). -- Best, Xuneng ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-01-28 02:49 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-02-26 06:59 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-03-21 10:29 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> @ 2026-03-23 07:56 ` Chao Li <[email protected]> 2026-03-23 08:41 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Chao Li @ 2026-03-23 07:56 UTC (permalink / raw) To: Xuneng Zhou <[email protected]>; Postgres hackers <[email protected]> > On Mar 21, 2026, at 18:29, Xuneng Zhou <[email protected]> wrote: > > On Thu, Mar 19, 2026 at 1:07 PM Chao Li <[email protected]> wrote: >> >> >> >>> On Feb 26, 2026, at 14:59, Chao Li <[email protected]> wrote: >>> >>> >>> >>>> On Jan 28, 2026, at 10:49, Chao Li <[email protected]> wrote: >>>> >>>> >>>> >>>>> On Jan 27, 2026, at 16:30, Chao Li <[email protected]> wrote: >>>>> >>>>> >>>>> >>>>>> On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: >>>>>>> >>>>>>> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: >>>>>>>> I found this bug while working on a related patch [1]. >>>>>>>> >>>>>>>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and >>>>>>>> that index is used as REPLICA IDENTITY on a partitioned table, the >>>>>>>> replica identity marking on partitions can be silently lost after the >>>>>>>> rebuild. >>>>>>> >>>>>>> I am slightly confused by the tests included in the proposed patch. >>>>>>> On HEAD, if I undo the proposed changes of tablecmds.c, the tests >>>>>>> pass. If I run the tests of the patch with the changes of >>>>>>> tablecmds.c, the tests also pass. >>>>>> >>>>>> Oops, that isn’t supposed to be so. I’ll check the test. >>>>>> >>>>> >>>>> Okay, I see the problem is here: >>>>> ``` >>>>> +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); >>>>> ``` >>>>> >>>>> I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. >>>>> >>>>> Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? >>>> >>>> I just updated the test to store index OIDs before and after rebuild into 2 temp tables, so that we can compare the OIDs to verify rebuild happens and replica identity preserved. >>>> >>>> I tried to port the test to master branch, and the test failed. From the test diff file, we can see replica identity lost on 3 leaf partitions: >>>> ``` >>>> @@ -360,9 +360,9 @@ >>>> ORDER BY b.index_name; >>>> index_name | rebuilt | ri_lost >>>> ---------------------------------------------------+---------+--------- >>>> - test_replica_identity_partitioned_p1_id_val_idx | t | f >>>> - test_replica_identity_partitioned_p2_1_id_val_idx | t | f >>>> - test_replica_identity_partitioned_p2_2_id_val_idx | t | f >>>> + test_replica_identity_partitioned_p1_id_val_idx | t | t >>>> + test_replica_identity_partitioned_p2_1_id_val_idx | t | t >>>> + test_replica_identity_partitioned_p2_2_id_val_idx | t | t >>>> test_replica_identity_partitioned_p2_id_val_idx | t | f >>>> test_replica_identity_partitioned_pkey | t | f >>>> (5 rows) >>>> ``` >>>> >>>> With this patch, the test passes and all replica identity are preserved. >>>> >>>> PFA v3: >>>> * Enhanced the test. >>>> * A small change in find_partition_replica_identity_indexes(): if we will not update a partition, then unlock it. >>>> >>>> Best regards, >>>> -- >>>> Chao Li (Evan) >>>> HighGo Software Co., Ltd. >>>> https://www.highgo.com/ >>>> >>>> >>>> >>>> >>>> <v3-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch> >>> >>> The CF asked for a rebase, thus rebased as v4. >>> > > > Hi, I reproduced this with the test case, and the patch appears > to resolve it. > > Some comments on v5: Thanks a lot for your review. > > -- Whether it makes sense to use a single list of pair structs instead > of two parallel OID lists (replicaIdentityIndexOids + > replicaIdentityTableOids) to avoid accidental desync. I don’t think that helps much. The current code of rebuilding index uses two lists changedIndexOids and changedIndexDefs. So, this patch matches the pattern of the existing code. > > -- It would be better to make lock handling in > find_partition_replica_identity_indexes() consistent > (relation_open(..., NoLock) if child is already locked, and avoid > mixed relation_close(..., lockmode)/NoLock behavior). That’s because if we are going to update a partition, then we need to hold the lock on the partition. > > -- Some typos in comments/tests (partion/parition). > Fixed. PFA v6: fixed a typo in comment. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v6-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch (21.6K, 2-v6-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch) download | inline diff: From a6cbf05aeb05883d196087e96562c51f26071e9d Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Tue, 27 Jan 2026 12:41:05 +0800 Subject: [PATCH v6] 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 <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Xuneng Zhou <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/tablecmds.c | 119 +++++++++++++----- .../regress/expected/replica_identity.out | 82 ++++++++++++ src/test/regress/sql/replica_identity.sql | 55 ++++++++ 3 files changed, 227 insertions(+), 29 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 219f604df7b..4d1d5d9c3ec 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -205,7 +205,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 */ @@ -662,9 +665,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); @@ -8761,7 +8764,7 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName, * Find everything that depends on the column (constraints, indexes, etc), * and record enough information to let us recreate the objects. */ - RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName); + RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName, lockmode); /* * Drop the dependency records of the GENERATED expression, in particular @@ -15080,7 +15083,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 @@ -15283,7 +15286,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]; @@ -15329,7 +15332,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) { @@ -15350,7 +15353,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, case ConstraintRelationId: Assert(foundObject.objectSubId == 0); - RememberConstraintForRebuilding(foundObject.objectId, tab); + RememberConstraintForRebuilding(foundObject.objectId, tab, lockmode); break; case ProcedureRelationId: @@ -15503,20 +15506,66 @@ 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)) + { + /* if we won't touch the partition, then unlock it */ + relation_close(partRel, lockmode); + 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); } /* @@ -15539,7 +15588,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 @@ -15584,7 +15633,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab) indoid = get_constraint_index(conoid); if (OidIsValid(indoid)) { - RememberReplicaIdentityForRebuilding(indoid, tab); + RememberReplicaIdentityForRebuilding(indoid, tab, lockmode); RememberClusterOnForRebuilding(indoid, tab); } } @@ -15595,7 +15644,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 @@ -15618,7 +15667,7 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) if (OidIsValid(conoid)) { - RememberConstraintForRebuilding(conoid, tab); + RememberConstraintForRebuilding(conoid, tab, lockmode); } else { @@ -15635,7 +15684,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); } } @@ -15814,19 +15863,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..8ff9da55569 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -290,6 +290,88 @@ 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); +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) + +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; + 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) + +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; + index_name | rebuilt | ri_lost +---------------------------------------------------+---------+--------- + test_replica_identity_partitioned_p1_id_val_idx | t | f + test_replica_identity_partitioned_p2_1_id_val_idx | t | f + test_replica_identity_partitioned_p2_2_id_val_idx | t | f + test_replica_identity_partitioned_p2_id_val_idx | t | f + test_replica_identity_partitioned_pkey | t | f +(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..e9a71eac8a9 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -134,6 +134,61 @@ 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); +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; +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; +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) ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-01-28 02:49 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-02-26 06:59 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-03-21 10:29 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 2026-03-23 07:56 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> @ 2026-03-23 08:41 ` Xuneng Zhou <[email protected]> 2026-03-24 07:25 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Xuneng Zhou @ 2026-03-23 08:41 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Postgres hackers <[email protected]> Hi, On Mon, Mar 23, 2026 at 3:57 PM Chao Li <[email protected]> wrote: > > > > > On Mar 21, 2026, at 18:29, Xuneng Zhou <[email protected]> wrote: > > > > On Thu, Mar 19, 2026 at 1:07 PM Chao Li <[email protected]> wrote: > >> > >> > >> > >>> On Feb 26, 2026, at 14:59, Chao Li <[email protected]> wrote: > >>> > >>> > >>> > >>>> On Jan 28, 2026, at 10:49, Chao Li <[email protected]> wrote: > >>>> > >>>> > >>>> > >>>>> On Jan 27, 2026, at 16:30, Chao Li <[email protected]> wrote: > >>>>> > >>>>> > >>>>> > >>>>>> On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: > >>>>>> > >>>>>> > >>>>>> > >>>>>>> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: > >>>>>>> > >>>>>>> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: > >>>>>>>> I found this bug while working on a related patch [1]. > >>>>>>>> > >>>>>>>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and > >>>>>>>> that index is used as REPLICA IDENTITY on a partitioned table, the > >>>>>>>> replica identity marking on partitions can be silently lost after the > >>>>>>>> rebuild. > >>>>>>> > >>>>>>> I am slightly confused by the tests included in the proposed patch. > >>>>>>> On HEAD, if I undo the proposed changes of tablecmds.c, the tests > >>>>>>> pass. If I run the tests of the patch with the changes of > >>>>>>> tablecmds.c, the tests also pass. > >>>>>> > >>>>>> Oops, that isn’t supposed to be so. I’ll check the test. > >>>>>> > >>>>> > >>>>> Okay, I see the problem is here: > >>>>> ``` > >>>>> +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); > >>>>> ``` > >>>>> > >>>>> I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. > >>>>> > >>>>> Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? > >>>> > >>>> I just updated the test to store index OIDs before and after rebuild into 2 temp tables, so that we can compare the OIDs to verify rebuild happens and replica identity preserved. > >>>> > >>>> I tried to port the test to master branch, and the test failed. From the test diff file, we can see replica identity lost on 3 leaf partitions: > >>>> ``` > >>>> @@ -360,9 +360,9 @@ > >>>> ORDER BY b.index_name; > >>>> index_name | rebuilt | ri_lost > >>>> ---------------------------------------------------+---------+--------- > >>>> - test_replica_identity_partitioned_p1_id_val_idx | t | f > >>>> - test_replica_identity_partitioned_p2_1_id_val_idx | t | f > >>>> - test_replica_identity_partitioned_p2_2_id_val_idx | t | f > >>>> + test_replica_identity_partitioned_p1_id_val_idx | t | t > >>>> + test_replica_identity_partitioned_p2_1_id_val_idx | t | t > >>>> + test_replica_identity_partitioned_p2_2_id_val_idx | t | t > >>>> test_replica_identity_partitioned_p2_id_val_idx | t | f > >>>> test_replica_identity_partitioned_pkey | t | f > >>>> (5 rows) > >>>> ``` > >>>> > >>>> With this patch, the test passes and all replica identity are preserved. > >>>> > >>>> PFA v3: > >>>> * Enhanced the test. > >>>> * A small change in find_partition_replica_identity_indexes(): if we will not update a partition, then unlock it. > >>>> > >>>> Best regards, > >>>> -- > >>>> Chao Li (Evan) > >>>> HighGo Software Co., Ltd. > >>>> https://www.highgo.com/ > >>>> > >>>> > >>>> > >>>> > >>>> <v3-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch> > >>> > >>> The CF asked for a rebase, thus rebased as v4. > >>> > > > > > > Hi, I reproduced this with the test case, and the patch appears > > to resolve it. > > > > Some comments on v5: > > Thanks a lot for your review. > > > > > -- Whether it makes sense to use a single list of pair structs instead > > of two parallel OID lists (replicaIdentityIndexOids + > > replicaIdentityTableOids) to avoid accidental desync. > > I don’t think that helps much. The current code of rebuilding index uses two lists changedIndexOids and changedIndexDefs. So, this patch matches the pattern of the existing code. > > > > > -- It would be better to make lock handling in > > find_partition_replica_identity_indexes() consistent > > (relation_open(..., NoLock) if child is already locked, and avoid > > mixed relation_close(..., lockmode)/NoLock behavior). > > That’s because if we are going to update a partition, then we need to hold the lock on the partition. There is one locking cleanup in find_partition_replica_identity_indexes(). find_inheritance_children(relId, lockmode) already acquires lockmode on every partition it returns, so I think the later relation_open() should use NoLock, not lockmode. For the same reason, all relation_close() calls in this function should use NoLock as well. Today the code does: partRel = relation_open(partRelOid, lockmode); ... relation_close(partRel, lockmode); That does not cause a correctness issue, because the lock manager reference-counts same-transaction acquisitions, so the lock remains held either way. But it is misleading: it suggests that relation_open() is where the partition lock is taken, and that the early relation_close(..., lockmode) is intentionally releasing it. Neither is actually true here, because the lock was already acquired by find_inheritance_children(). So I think this should be adjusted to: partRel = relation_open(partRelOid, NoLock); and all close sites in this function should be: relation_close(partRel, NoLock); The comment on the early-close path should also be updated, since it is not really unlocking the partition. Something like "No matching partition index; just close the relcache entry" would match the actual behavior better. > > > > -- Some typos in comments/tests (partion/parition). > > > > Fixed. > > PFA v6: fixed a typo in comment. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > > -- Best, Xuneng ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-01-28 02:49 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-02-26 06:59 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-03-21 10:29 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 2026-03-23 07:56 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-03-23 08:41 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> @ 2026-03-24 07:25 ` Chao Li <[email protected]> 2026-04-06 06:04 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Chao Li @ 2026-03-24 07:25 UTC (permalink / raw) To: Xuneng Zhou <[email protected]>; +Cc: Postgres hackers <[email protected]> > On Mar 23, 2026, at 16:41, Xuneng Zhou <[email protected]> wrote: > > Hi, > > On Mon, Mar 23, 2026 at 3:57 PM Chao Li <[email protected]> wrote: > > > > > > > > > On Mar 21, 2026, at 18:29, Xuneng Zhou <[email protected]> wrote: > > > > > > On Thu, Mar 19, 2026 at 1:07 PM Chao Li <[email protected]> wrote: > > >> > > >> > > >> > > >>> On Feb 26, 2026, at 14:59, Chao Li <[email protected]> wrote: > > >>> > > >>> > > >>> > > >>>> On Jan 28, 2026, at 10:49, Chao Li <[email protected]> wrote: > > >>>> > > >>>> > > >>>> > > >>>>> On Jan 27, 2026, at 16:30, Chao Li <[email protected]> wrote: > > >>>>> > > >>>>> > > >>>>> > > >>>>>> On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>>> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: > > >>>>>>> > > >>>>>>> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: > > >>>>>>>> I found this bug while working on a related patch [1]. > > >>>>>>>> > > >>>>>>>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and > > >>>>>>>> that index is used as REPLICA IDENTITY on a partitioned table, the > > >>>>>>>> replica identity marking on partitions can be silently lost after the > > >>>>>>>> rebuild. > > >>>>>>> > > >>>>>>> I am slightly confused by the tests included in the proposed patch. > > >>>>>>> On HEAD, if I undo the proposed changes of tablecmds.c, the tests > > >>>>>>> pass. If I run the tests of the patch with the changes of > > >>>>>>> tablecmds.c, the tests also pass. > > >>>>>> > > >>>>>> Oops, that isn’t supposed to be so. I’ll check the test. > > >>>>>> > > >>>>> > > >>>>> Okay, I see the problem is here: > > >>>>> ``` > > >>>>> +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); > > >>>>> ``` > > >>>>> > > >>>>> I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. > > >>>>> > > >>>>> Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? > > >>>> > > >>>> I just updated the test to store index OIDs before and after rebuild into 2 temp tables, so that we can compare the OIDs to verify rebuild happens and replica identity preserved. > > >>>> > > >>>> I tried to port the test to master branch, and the test failed. From the test diff file, we can see replica identity lost on 3 leaf partitions: > > >>>> ``` > > >>>> @@ -360,9 +360,9 @@ > > >>>> ORDER BY b.index_name; > > >>>> index_name | rebuilt | ri_lost > > >>>> ---------------------------------------------------+---------+--------- > > >>>> - test_replica_identity_partitioned_p1_id_val_idx | t | f > > >>>> - test_replica_identity_partitioned_p2_1_id_val_idx | t | f > > >>>> - test_replica_identity_partitioned_p2_2_id_val_idx | t | f > > >>>> + test_replica_identity_partitioned_p1_id_val_idx | t | t > > >>>> + test_replica_identity_partitioned_p2_1_id_val_idx | t | t > > >>>> + test_replica_identity_partitioned_p2_2_id_val_idx | t | t > > >>>> test_replica_identity_partitioned_p2_id_val_idx | t | f > > >>>> test_replica_identity_partitioned_pkey | t | f > > >>>> (5 rows) > > >>>> ``` > > >>>> > > >>>> With this patch, the test passes and all replica identity are preserved. > > >>>> > > >>>> PFA v3: > > >>>> * Enhanced the test. > > >>>> * A small change in find_partition_replica_identity_indexes(): if we will not update a partition, then unlock it. > > >>>> > > >>>> Best regards, > > >>>> -- > > >>>> Chao Li (Evan) > > >>>> HighGo Software Co., Ltd. > > >>>> https://www.highgo.com/ > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> <v3-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch> > > >>> > > >>> The CF asked for a rebase, thus rebased as v4. > > >>> > > > > > > > > > Hi, I reproduced this with the test case, and the patch appears > > > to resolve it. > > > > > > Some comments on v5: > > > > Thanks a lot for your review. > > > > > > > > -- Whether it makes sense to use a single list of pair structs instead > > > of two parallel OID lists (replicaIdentityIndexOids + > > > replicaIdentityTableOids) to avoid accidental desync. > > > > I don’t think that helps much. The current code of rebuilding index uses two lists changedIndexOids and changedIndexDefs. So, this patch matches the pattern of the existing code. > > > > > > > > -- It would be better to make lock handling in > > > find_partition_replica_identity_indexes() consistent > > > (relation_open(..., NoLock) if child is already locked, and avoid > > > mixed relation_close(..., lockmode)/NoLock behavior). > > > > That’s because if we are going to update a partition, then we need to hold the lock on the partition. > > There is one locking cleanup in find_partition_replica_identity_indexes(). > > find_inheritance_children(relId, lockmode) already acquires lockmode on > every partition it returns, so I think the later relation_open() should use > NoLock, not lockmode. For the same reason, all relation_close() calls in > this function should use NoLock as well. > > Today the code does: > > partRel =relation_open(partRelOid, lockmode); > ... > relation_close(partRel, lockmode); > > That does not cause a correctness issue, because the lock manager > reference-counts same-transaction acquisitions, so the lock remains held > either way. But it is misleading: it suggests that relation_open() is where > the partition lock is taken, and that the early relation_close(..., lockmode) > is intentionally releasing it. Neither is actually true here, because the lock > was already acquired by find_inheritance_children(). > > So I think this should be adjusted to: > > partRel = relation_open(partRelOid, NoLock); > > and all close sites in this function should be: > > relation_close(partRel, NoLock); > > The comment on the early-close path should also be updated, since it is not > really unlocking the partition. Something like "No matching partition index; > just close the relcache entry" would match the actual behavior better. > Okay, in find_partition_replica_identity_indexes, we can use NOLOCK to open partitions as they have been locked by find_inheritance_children. But for those partitions that we won’t touch, we still want to unlock them. PFA v7. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v7-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch (21.6K, 2-v7-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch) download | inline diff: From 8727adecb1afbe1e16c3a1844f32be962a656e05 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Tue, 27 Jan 2026 12:41:05 +0800 Subject: [PATCH v7] 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 <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Xuneng Zhou <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/tablecmds.c | 119 +++++++++++++----- .../regress/expected/replica_identity.out | 82 ++++++++++++ src/test/regress/sql/replica_identity.sql | 55 ++++++++ 3 files changed, 227 insertions(+), 29 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index c69c12dc014..129e2e5f553 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -205,7 +205,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 */ @@ -662,9 +665,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); @@ -8761,7 +8764,7 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName, * Find everything that depends on the column (constraints, indexes, etc), * and record enough information to let us recreate the objects. */ - RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName); + RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName, lockmode); /* * Drop the dependency records of the GENERATED expression, in particular @@ -15080,7 +15083,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 @@ -15283,7 +15286,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]; @@ -15329,7 +15332,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) { @@ -15350,7 +15353,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, case ConstraintRelationId: Assert(foundObject.objectSubId == 0); - RememberConstraintForRebuilding(foundObject.objectId, tab); + RememberConstraintForRebuilding(foundObject.objectId, tab, lockmode); break; case ProcedureRelationId: @@ -15503,20 +15506,66 @@ 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, NoLock); + + partIndexId = index_get_partition(partRel, indexId); + if (!OidIsValid(partIndexId)) + { + /* if we won't touch the partition, then unlock it */ + relation_close(partRel, lockmode); + 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); } /* @@ -15539,7 +15588,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 @@ -15584,7 +15633,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab) indoid = get_constraint_index(conoid); if (OidIsValid(indoid)) { - RememberReplicaIdentityForRebuilding(indoid, tab); + RememberReplicaIdentityForRebuilding(indoid, tab, lockmode); RememberClusterOnForRebuilding(indoid, tab); } } @@ -15595,7 +15644,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 @@ -15618,7 +15667,7 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) if (OidIsValid(conoid)) { - RememberConstraintForRebuilding(conoid, tab); + RememberConstraintForRebuilding(conoid, tab, lockmode); } else { @@ -15635,7 +15684,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); } } @@ -15814,19 +15863,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..8ff9da55569 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -290,6 +290,88 @@ 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); +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) + +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; + 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) + +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; + index_name | rebuilt | ri_lost +---------------------------------------------------+---------+--------- + test_replica_identity_partitioned_p1_id_val_idx | t | f + test_replica_identity_partitioned_p2_1_id_val_idx | t | f + test_replica_identity_partitioned_p2_2_id_val_idx | t | f + test_replica_identity_partitioned_p2_id_val_idx | t | f + test_replica_identity_partitioned_pkey | t | f +(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..e9a71eac8a9 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -134,6 +134,61 @@ 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); +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; +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; +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) ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-01-28 02:49 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-02-26 06:59 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-03-21 10:29 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 2026-03-23 07:56 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-03-23 08:41 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 2026-03-24 07:25 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> @ 2026-04-06 06:04 ` Xuneng Zhou <[email protected]> 2026-04-14 03:13 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Xuneng Zhou @ 2026-04-06 06:04 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Postgres hackers <[email protected]> On Tue, Mar 24, 2026 at 3:26 PM Chao Li <[email protected]> wrote: > > > > > On Mar 23, 2026, at 16:41, Xuneng Zhou <[email protected]> wrote: > > > > Hi, > > > > On Mon, Mar 23, 2026 at 3:57 PM Chao Li <[email protected]> wrote: > > > > > > > > > > > > > On Mar 21, 2026, at 18:29, Xuneng Zhou <[email protected]> wrote: > > > > > > > > On Thu, Mar 19, 2026 at 1:07 PM Chao Li <[email protected]> wrote: > > > >> > > > >> > > > >> > > > >>> On Feb 26, 2026, at 14:59, Chao Li <[email protected]> wrote: > > > >>> > > > >>> > > > >>> > > > >>>> On Jan 28, 2026, at 10:49, Chao Li <[email protected]> wrote: > > > >>>> > > > >>>> > > > >>>> > > > >>>>> On Jan 27, 2026, at 16:30, Chao Li <[email protected]> wrote: > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>>> On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>>> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: > > > >>>>>>> > > > >>>>>>> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: > > > >>>>>>>> I found this bug while working on a related patch [1]. > > > >>>>>>>> > > > >>>>>>>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and > > > >>>>>>>> that index is used as REPLICA IDENTITY on a partitioned table, the > > > >>>>>>>> replica identity marking on partitions can be silently lost after the > > > >>>>>>>> rebuild. > > > >>>>>>> > > > >>>>>>> I am slightly confused by the tests included in the proposed patch. > > > >>>>>>> On HEAD, if I undo the proposed changes of tablecmds.c, the tests > > > >>>>>>> pass. If I run the tests of the patch with the changes of > > > >>>>>>> tablecmds.c, the tests also pass. > > > >>>>>> > > > >>>>>> Oops, that isn’t supposed to be so. I’ll check the test. > > > >>>>>> > > > >>>>> > > > >>>>> Okay, I see the problem is here: > > > >>>>> ``` > > > >>>>> +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); > > > >>>>> ``` > > > >>>>> > > > >>>>> I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. > > > >>>>> > > > >>>>> Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? > > > >>>> > > > >>>> I just updated the test to store index OIDs before and after rebuild into 2 temp tables, so that we can compare the OIDs to verify rebuild happens and replica identity preserved. > > > >>>> > > > >>>> I tried to port the test to master branch, and the test failed. ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: tablecmds: fix bug where index rebuild loses replica identity on partitions 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-01-28 02:49 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-02-26 06:59 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-03-21 10:29 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 2026-03-23 07:56 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-03-23 08:41 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> 2026-03-24 07:25 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-04-06 06:04 ` Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Xuneng Zhou <[email protected]> @ 2026-04-14 03:13 ` Chao Li <[email protected]> 0 siblings, 0 replies; 9+ messages in thread From: Chao Li @ 2026-04-14 03:13 UTC (permalink / raw) To: Xuneng Zhou <[email protected]>; +Cc: Postgres hackers <[email protected]> > On Apr 6, 2026, at 14:04, Xuneng Zhou <[email protected]> wrote: > > On Tue, Mar 24, 2026 at 3:26 PM Chao Li <[email protected]> wrote: >> >> >> >>> On Mar 23, 2026, at 16:41, Xuneng Zhou <[email protected]> wrote: >>> >>> Hi, >>> >>> On Mon, Mar 23, 2026 at 3:57 PM Chao Li <[email protected]> wrote: >>>> >>>> >>>> >>>>> On Mar 21, 2026, at 18:29, Xuneng Zhou <[email protected]> wrote: >>>>> >>>>> On Thu, Mar 19, 2026 at 1:07 PM Chao Li <[email protected]> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> On Feb 26, 2026, at 14:59, Chao Li <[email protected]> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On Jan 28, 2026, at 10:49, Chao Li <[email protected]> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On Jan 27, 2026, at 16:30, Chao Li <[email protected]> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On Jan 27, 2026, at 15:59, Chao Li <[email protected]> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> On Jan 27, 2026, at 15:39, Michael Paquier <[email protected]> wrote: >>>>>>>>>>> >>>>>>>>>>> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote: >>>>>>>>>>>> I found this bug while working on a related patch [1]. >>>>>>>>>>>> >>>>>>>>>>>> When ALTER TABLE ... ALTER COLUMN TYPE causes an index rebuild, and >>>>>>>>>>>> that index is used as REPLICA IDENTITY on a partitioned table, the >>>>>>>>>>>> replica identity marking on partitions can be silently lost after the >>>>>>>>>>>> rebuild. >>>>>>>>>>> >>>>>>>>>>> I am slightly confused by the tests included in the proposed patch. >>>>>>>>>>> On HEAD, if I undo the proposed changes of tablecmds.c, the tests >>>>>>>>>>> pass. If I run the tests of the patch with the changes of >>>>>>>>>>> tablecmds.c, the tests also pass. >>>>>>>>>> >>>>>>>>>> Oops, that isn’t supposed to be so. I’ll check the test. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Okay, I see the problem is here: >>>>>>>>> ``` >>>>>>>>> +CREATE UNIQUE INDEX test_replica_identity_partitioned_pkey ON test_replica_identity_partitioned (id); >>>>>>>>> ``` >>>>>>>>> >>>>>>>>> I missed to add column “val” into the index, so that alter type of val didn’t cause index rebuild. >>>>>>>>> >>>>>>>>> Ideally, it’s better to also verify that index OIDs should have changed before and after alter column type, but I haven’t figured out how to do so. Do you have an idea? >>>>>>>> >>>>>>>> I just updated the test to store index OIDs before and after rebuild into 2 temp tables, so that we can compare the OIDs to verify rebuild happens and replica identity preserved. >>>>>>>> >>>>>>>> I tried to port the test to master branch, and the test failed. From the test diff file, we can see replica identity lost on 3 leaf partitions: >>>>>>>> ``` >>>>>>>> @@ -360,9 +360,9 @@ >>>>>>>> ORDER BY b.index_name; >>>>>>>> index_name | rebuilt | ri_lost >>>>>>>> ---------------------------------------------------+---------+--------- >>>>>>>> - test_replica_identity_partitioned_p1_id_val_idx | t | f >>>>>>>> - test_replica_identity_partitioned_p2_1_id_val_idx | t | f >>>>>>>> - test_replica_identity_partitioned_p2_2_id_val_idx | t | f >>>>>>>> + test_replica_identity_partitioned_p1_id_val_idx | t | t >>>>>>>> + test_replica_identity_partitioned_p2_1_id_val_idx | t | t >>>>>>>> + test_replica_identity_partitioned_p2_2_id_val_idx | t | t >>>>>>>> test_replica_identity_partitioned_p2_id_val_idx | t | f >>>>>>>> test_replica_identity_partitioned_pkey | t | f >>>>>>>> (5 rows) >>>>>>>> ``` >>>>>>>> >>>>>>>> With this patch, the test passes and all replica identity are preserved. >>>>>>>> >>>>>>>> PFA v3: >>>>>>>> * Enhanced the test. >>>>>>>> * A small change in find_partition_replica_identity_indexes(): if we will not update a partition, then unlock it. >>>>>>>> >>>>>>>> Best regards, >>>>>>>> -- >>>>>>>> Chao Li (Evan) >>>>>>>> HighGo Software Co., Ltd. >>>>>>>> https://www.highgo.com/ >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> <v3-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch> >>>>>>> >>>>>>> The CF asked for a rebase, thus rebased as v4. >>>>>>> >>>>> >>>>> >>>>> Hi, I reproduced this with the test case, and the patch appears >>>>> to resolve it. >>>>> >>>>> Some comments on v5: >>>> >>>> Thanks a lot for your review. >>>> >>>>> >>>>> -- Whether it makes sense to use a single list of pair structs instead >>>>> of two parallel OID lists (replicaIdentityIndexOids + >>>>> replicaIdentityTableOids) to avoid accidental desync. >>>> >>>> I don’t think that helps much. The current code of rebuilding index uses two lists changedIndexOids and changedIndexDefs. So, this patch matches the pattern of the existing code. >>>> >>>>> >>>>> -- It would be better to make lock handling in >>>>> find_partition_replica_identity_indexes() consistent >>>>> (relation_open(..., NoLock) if child is already locked, and avoid >>>>> mixed relation_close(..., lockmode)/NoLock behavior). >>>> >>>> That’s because if we are going to update a partition, then we need to hold the lock on the partition. >>> >>> There is one locking cleanup in find_partition_replica_identity_indexes(). >>> >>> find_inheritance_children(relId, lockmode) already acquires lockmode on >>> every partition it returns, so I think the later relation_open() should use >>> NoLock, not lockmode. For the same reason, all relation_close() calls in >>> this function should use NoLock as well. >>> >>> Today the code does: >>> >>> partRel =relation_open(partRelOid, lockmode); >>> ... >>> relation_close(partRel, lockmode); >>> >>> That does not cause a correctness issue, because the lock manager >>> reference-counts same-transaction acquisitions, so the lock remains held >>> either way. But it is misleading: it suggests that relation_open() is where >>> the partition lock is taken, and that the early relation_close(..., lockmode) >>> is intentionally releasing it. Neither is actually true here, because the lock >>> was already acquired by find_inheritance_children(). >>> >>> So I think this should be adjusted to: >>> >>> partRel = relation_open(partRelOid, NoLock); >>> >>> and all close sites in this function should be: >>> >>> relation_close(partRel, NoLock); >>> >>> The comment on the early-close path should also be updated, since it is not >>> really unlocking the partition. Something like "No matching partition index; >>> just close the relcache entry" would match the actual behavior better. >>> >> >> Okay, in find_partition_replica_identity_indexes, we can use NOLOCK to open partitions as they have been locked by find_inheritance_children. But for those partitions that we won’t touch, we still want to unlock them. >> >> PFA v7. >> > > v7 LGTM. > > -- > Best, > Xuneng Rebased as v8. Nothing changed. Per [1], to participate Peter E.’s “in-person commitfest” session, I am adding tag “PGConf.dev” to the CF entry: https://commitfest.postgresql.org/patch/6440/ [1] https://postgr.es/m/[email protected] Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v8-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch (21.6K, 2-v8-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch) download | inline diff: From 6819d67b0905e5c56337ece8ae38ea0477f16951 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Tue, 27 Jan 2026 12:41:05 +0800 Subject: [PATCH v8] 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 <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Xuneng Zhou <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/tablecmds.c | 119 +++++++++++++----- .../regress/expected/replica_identity.out | 82 ++++++++++++ src/test/regress/sql/replica_identity.sql | 55 ++++++++ 3 files changed, 227 insertions(+), 29 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index eec09ba1ded..26999c8ad0b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -205,7 +205,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 */ @@ -662,9 +665,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); @@ -8763,7 +8766,7 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName, * Find everything that depends on the column (constraints, indexes, etc), * and record enough information to let us recreate the objects. */ - RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName); + RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName, lockmode); /* * Drop the dependency records of the GENERATED expression, in particular @@ -15084,7 +15087,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 @@ -15287,7 +15290,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]; @@ -15333,7 +15336,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) { @@ -15354,7 +15357,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, case ConstraintRelationId: Assert(foundObject.objectSubId == 0); - RememberConstraintForRebuilding(foundObject.objectId, tab); + RememberConstraintForRebuilding(foundObject.objectId, tab, lockmode); break; case ProcedureRelationId: @@ -15507,20 +15510,66 @@ 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, NoLock); + + partIndexId = index_get_partition(partRel, indexId); + if (!OidIsValid(partIndexId)) + { + /* if we won't touch the partition, then unlock it */ + relation_close(partRel, lockmode); + 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); } /* @@ -15543,7 +15592,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 @@ -15588,7 +15637,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab) indoid = get_constraint_index(conoid); if (OidIsValid(indoid)) { - RememberReplicaIdentityForRebuilding(indoid, tab); + RememberReplicaIdentityForRebuilding(indoid, tab, lockmode); RememberClusterOnForRebuilding(indoid, tab); } } @@ -15599,7 +15648,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 @@ -15622,7 +15671,7 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) if (OidIsValid(conoid)) { - RememberConstraintForRebuilding(conoid, tab); + RememberConstraintForRebuilding(conoid, tab, lockmode); } else { @@ -15639,7 +15688,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); } } @@ -15818,19 +15867,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 336b04fa278..4f363b88090 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -292,6 +292,88 @@ 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); +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) + +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; + 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) + +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; + index_name | rebuilt | ri_lost +---------------------------------------------------+---------+--------- + test_replica_identity_partitioned_p1_id_val_idx | t | f + test_replica_identity_partitioned_p2_1_id_val_idx | t | f + test_replica_identity_partitioned_p2_2_id_val_idx | t | f + test_replica_identity_partitioned_p2_id_val_idx | t | f + test_replica_identity_partitioned_pkey | t | f +(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..e9a71eac8a9 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -134,6 +134,61 @@ 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); +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; +-- Create a temp table to store the index OIDs before rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_before AS +SELECT c.oid AS index_oid, c.relname 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; +-- Create a temp table to store the index OIDs after rebuild +CREATE TEMP TABLE test_replica_identity_partitioned_temp_index_oids_after AS +SELECT c.oid AS index_oid, c.relname 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; +-- Compare the before and after to ensure replica identity preserved +SELECT b.index_name, b.index_oid != a.index_oid as rebuilt, b.indisreplident != a.indisreplident AS ri_lost + FROM test_replica_identity_partitioned_temp_index_oids_before b + JOIN test_replica_identity_partitioned_temp_index_oids_after a + ON b.index_name = a.index_name + ORDER BY b.index_name; +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) ^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2026-04-14 03:13 UTC | newest] Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-01-27 08:30 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions Chao Li <[email protected]> 2026-01-28 02:49 ` Chao Li <[email protected]> 2026-02-26 06:59 ` Chao Li <[email protected]> 2026-03-21 10:29 ` Xuneng Zhou <[email protected]> 2026-03-23 07:56 ` Chao Li <[email protected]> 2026-03-23 08:41 ` Xuneng Zhou <[email protected]> 2026-03-24 07:25 ` Chao Li <[email protected]> 2026-04-06 06:04 ` Xuneng Zhou <[email protected]> 2026-04-14 03:13 ` Chao Li <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox