Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.300.41.1.7\))
Subject: Re: tablecmds: fix bug where index rebuild loses replica identity on
 partitions
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <aXhruFBS8ZZgKolq@paquier.xyz>
Date: Tue, 27 Jan 2026 15:59:01 +0800
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <0B38B9DC-A53C-437C-B9BD-807F30A032BA@gmail.com>
References: <DB533C25-C6BA-4C0F-8046-96168E9CDD72@gmail.com>
 <aXhruFBS8ZZgKolq@paquier.xyz>
To: Michael Paquier <michael@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/0B38B9DC-A53C-437C-B9BD-807F30A032BA%40gmail.com>
Precedence: bulk


> On Jan 27, 2026, at 15:39, Michael Paquier <michael@paquier.xyz> =
wrote:
>=20
> On Tue, Jan 27, 2026 at 01:13:32PM +0800, Chao Li wrote:
>> I found this bug while working on a related patch [1].
>>=20
>> 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.
>=20
> 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. =20

Oops, that isn=E2=80=99t supposed to be so. I=E2=80=99ll check the test.

> These tests don't check what you
> want them to.
>=20
> -    if (tab->replicaIdentityIndex)
> +    if (tab->replicaIdentityIndexOids !=3D NIL)
>         elog(ERROR, "relation %u has multiple indexes marked as =
replica identity", tab->relid);
>=20
> 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=E2=80=99t =
return the rebuilt index OIDs to =E2=80=9Calter table=E2=80=9D.

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/