Re: tablecmds: fix bug where index rebuild loses replica identity on partitions

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Alberto Piai <alberto(dot)piai(at)gmail(dot)com>
Cc: Sami Imseih <samimseih(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Xuneng Zhou <xunengzhou(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Robert Treat <rob(at)xzilla(dot)net>
Subject: Re: tablecmds: fix bug where index rebuild loses replica identity on partitions
Date: 2026-07-28 08:46:50
Message-ID: EFBAC69A-E37C-4A89-AE6F-28F2A1C5F46F@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Jul 28, 2026, at 14:50, Alberto Piai <alberto(dot)piai(at)gmail(dot)com> wrote:
>
> On Tue Jul 28, 2026 at 1:11 AM CEST, Sami Imseih wrote:
>>> Also, this issue is not limited to replica identity. CLUSTER ON does
>>> not copy/restore indisclustered on partition indexes at all, and this
>>> should be addressed as well, right?
>>
>> Robert (cc'd) reminded me offline that REPACK ... USING INDEX ==
>> CLUSTER ON.
>
> Hi Sami,
>
> have you seen my email from a few days ago?
>
> https://www.postgresql.org/message-id/DK5H6VM4U4J4.C8APP2ZH2CAT%40gmail.com
>
> I'm pretty sure the root cause is not addressed by patch v9.
>
> If my analysis is correct (RememberIndexForRebuilding not being called
> on the child index because of an earlier drop), that would also explain
> the issues with CLUSTER (As RememberIndexForRebuilding calls both
> RememberReplicaIdentityForRebuilding and
> RememberClusterOnForRebuilding).
>
> In that case, rather than tracking and restoring more state, I think a
> better fix would be to make sure that ATPostAlterTypeCleanup doesn't
> find the child table in a half-cleaned-up state. Then
> RememberIndexForRebuilding would be called and the replica identity and
> cluster state would be restored correctly.
>

Hi Alberto,

Thanks for your review and suggestion. I think your analysis is correct, and your suggested direction sounds reasonable. However, the current ALTER TABLE infra doesn't seem well suited to it. ATPostAlterTypeCleanup() for the parent calls performMultipleDeletions(), which also deletes the dependent child partition indexes, while DefineIndex() for the parent automatically recreates the parent index and all child indexes.

Your direction would allow each child partition to remember its own index properties so that they can be restored afterward. I can see two possible approaches:

1) Make the parent and each child responsible for deleting and rebuilding their own indexes.

2) Move performMultipleDeletions() out of ATPostAlterTypeCleanup() to a later stage, after all children have recorded their dependent indexes, and coordinate recreation to avoid rebuilding child indexes twice.

I’m afraid both approaches would require significant refactoring of the current ALTER TABLE infra, which would go beyond the scope of this patch, as it only aims to fix a bug.

If we want to pursue the direction you suggested, I think that would be better handled as a separate refactoring patch.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2026-07-28 08:48:17 Re: Support EXCEPT for TABLES IN SCHEMA publications
Previous Message Chauhan Dhruv 2026-07-28 08:46:43 Re: Expression index can get an empty generated column name