Обсуждение: tablecmds: fix bug where index rebuild loses replica identity on partitions

Hi Hackers,

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
partitionedtable, the replica 
identity marking on partitions can be silently lost after the rebuild.

Below is a simple reproduction:
```
-- create a partitioned table and a parition
create table parent (id int not null, val int not null) partition by range (id);
create table child partition of parent for values from (1) to (100);

-- create an index on parent
create unique index indx_1 on parent(id, val);

-- the index is auto created on child, and both indexes’ indisreplident are false
select c.relname as index_name, c.oid as index_oid, i.indisreplident from pg_class c join pg_index i on c.oid =
i.indexrelidwhere (c.relname = 'indx_1' or c.relname = 'child_id_val_idx’); 
    index_name    | index_oid | indisreplident
------------------+-----------+----------------
 indx_1           |     24594 | f
 child_id_val_idx |     24595 | f
(2 rows)

-- as replica identity doesn’t recurse, set it on parent and child individually
alter table parent replica identity using index indx_1;
alter table child replica identity using index child_id_val_idx;

-- now both indexes are marked as replica identity
select c.relname as index_name, c.oid as index_oid, i.indisreplident from pg_class c join pg_index i on c.oid =
i.indexrelidwhere (c.relname = 'indx_1' or c.relname = ‘child_id_val_idx'); 
    index_name    | index_oid | indisreplident
------------------+-----------+----------------
 indx_1           |     24594 | t
 child_id_val_idx |     24595 | t
(2 rows)

-- alter a column type, the column is part of the index, it will cause the index to rebuid
alter table parent alter val type bigint;

-- from the OIDs, we can see both indexes are rebuilt, but the child partition loses its replica identity marking
select c.relname as index_name, c.oid as index_oid, i.indisreplident from pg_class c join pg_index i on c.oid =
i.indexrelidwhere (c.relname = 'indx_1' or c.relname = ‘child_id_val_idx'); 
    index_name    | index_oid | indisreplident
------------------+-----------+----------------
 child_id_val_idx |     24597 | f
 indx_1           |     24596 | t
(2 rows)
```

This patch fixes the bug by tracking replica identity indexes across partition hierarchies and restoring replica
identitymarkings on all affected partitions after index rebuilds.  Regression tests are added. 

[1] https://postgr.es/m/CAEoWx2nJ71hy8R614HQr7vQhkBReO9AANPODPg0aSQs74eOdLQ@mail.gmail.com

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

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

> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> 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/

> On Jan 27, 2026, at 15:59, Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
>> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> 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’tfigured 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/

> On Jan 27, 2026, at 16:30, Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
>> On Jan 27, 2026, at 15:59, Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> 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’tfigured 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
OIDsto 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
loston 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/

> On Jan 28, 2026, at 10:49, Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
>> On Jan 27, 2026, at 16:30, Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Jan 27, 2026, at 15:59, Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>>
>>>
>>>> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> 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’tfigured 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
OIDsto 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
loston 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/

> On Feb 26, 2026, at 14:59, Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
>> On Jan 28, 2026, at 10:49, Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Jan 27, 2026, at 16:30, Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>>
>>>
>>>> On Jan 27, 2026, at 15:59, Chao Li <li.evan.chao@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> 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’tfigured 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
OIDsto 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
loston 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/
>
>
>
>
> <v4-0001-tablecmds-fix-bug-where-index-rebuild-loses-repli.patch>

Rebased, and a gentle ping.

Since this is a bug fix and the issue is easy to reproduce, I’m hoping it can still make it into v19.

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

On Thu, Mar 19, 2026 at 1:07 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> > On Feb 26, 2026, at 14:59, Chao Li <li.evan.chao@gmail.com> wrote:
> >
> >
> >
> >> On Jan 28, 2026, at 10:49, Chao Li <li.evan.chao@gmail.com> wrote:
> >>
> >>
> >>
> >>> On Jan 27, 2026, at 16:30, Chao Li <li.evan.chao@gmail.com> wrote:
> >>>
> >>>
> >>>
> >>>> On Jan 27, 2026, at 15:59, Chao Li <li.evan.chao@gmail.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>>> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> 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’tfigured 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
theOIDs 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
identitylost 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

> On Mar 21, 2026, at 18:29, Xuneng Zhou <xunengzhou@gmail.com> wrote:
>
> On Thu, Mar 19, 2026 at 1:07 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Feb 26, 2026, at 14:59, Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>>
>>>
>>>> On Jan 28, 2026, at 10:49, Chao Li <li.evan.chao@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Jan 27, 2026, at 16:30, Chao Li <li.evan.chao@gmail.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> On Jan 27, 2026, at 15:59, Chao Li <li.evan.chao@gmail.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> 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’tfigured 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
theOIDs 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
identitylost 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/

> On Mar 23, 2026, at 16:41, Xuneng Zhou <xunengzhou@gmail.com> wrote:
>
> Hi,
>
> On Mon, Mar 23, 2026 at 3:57 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >
> >
> >
> > > On Mar 21, 2026, at 18:29, Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > >
> > > On Thu, Mar 19, 2026 at 1:07 PM Chao Li <li.evan.chao@gmail.com> wrote:
> > >>
> > >>
> > >>
> > >>> On Feb 26, 2026, at 14:59, Chao Li <li.evan.chao@gmail.com> wrote:
> > >>>
> > >>>
> > >>>
> > >>>> On Jan 28, 2026, at 10:49, Chao Li <li.evan.chao@gmail.com> wrote:
> > >>>>
> > >>>>
> > >>>>
> > >>>>> On Jan 27, 2026, at 16:30, Chao Li <li.evan.chao@gmail.com> wrote:
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>> On Jan 27, 2026, at 15:59, Chao Li <li.evan.chao@gmail.com> wrote:
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> 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,
butI 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
comparethe 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
identitylost 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/