> 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.

> These tests don't check what you
> want them to.
> 
> -    if (tab->replicaIdentityIndex)
> +    if (tab->replicaIdentityIndexOids != NIL)
>         elog(ERROR, "relation %u has multiple indexes marked as replica 
> identity", tab->relid);
> 
> This looks wrong to me.  This new list tracks the OIDs of indexes
> where you'd want to make sure that relreplident is updated, and it
> could be possible, based on your proposal, that multiple indexes are
> stored in this list.  The error message is at least not in line
> anymore.  Actually, do we really need this extra list at all?  The
> list of indexes to rebuild are tracked already in changedIndexOids,
> and the partitioned indexes seem to be in it, no?

No, changedIndexOids only tracks the root index OID. DefineIndex() will 
automatically rebuild indexes on all partitions, and won’t return the rebuilt 
index OIDs to “alter table”.

So, before index rebuild, this patch records potential affected indexes in 
replicaIdentityIndexOids, and after index rebuild, restore replica identity on 
them.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Reply via email to