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

> On Aug 27, 2026, at 13:38, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
>> On Aug 27, 2026, at 11:42, Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>>
>>
>> Hi,
>>
>> On Wednesday, August 26, 2026 1:30 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>> I reviewed v14 and found that a leaf index’s tablespace can also be lost and
>>> needs to be restored. I fixed that in v15-0002.
>>>
>>> I also addressed Sami’s review comments in v15, except for the hash-table
>>> suggestion. I tried using an HTAB, but ran into several build failures. I didn’t
>>> want to spend more time on it, so I gave up on that approach.
>>
>> I have few thoughts for the patch:
>
> Hi Zhijie,
>
> Thank you very much for the review.
>
>>
>> 1.
>>
>> I noticed that the code updating idxisclustered and idxisreplident looks a bit
>> unusual to me. These updates are placed inside DefineIndex(), where a fresh
>> pg_index and pg_class row are already being created for the new index. It seems
>> a bit confusing that the patch also performs post-updates for these rows within
>> the same function.
>>
>> Perhaps we could add a comment explaining why these values can't be set directly
>> when creating the new rows - that would help future readers understand the
>> reasoning.
>
> Updated the comment in v15.
>
>>
>> I'm also concerned that updating the flag this way won't fire the
>> ObjectPostAlterHook (as Sami also mentioned). I think some analysis or comments
>> are worth adding here.
>
> Added the hook call in v15.
>
>>
>> Few nits:
>>
>> 2. I think this should report an ERROR, since we don't expect the tuple to be
>> concurrently updated, IIUC.
>>
>> classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(leafIndexOid));
>> if (!HeapTupleIsValid(classtup))
>> {
>> ReleaseSysCache(idxtup);
>> continue;
>> }
>
> Agreed and fixed in v15.
>
>>
>> 3. It's unnecessary to set stattargets = NIL when you'll overwrite it right
>> after anyway.
>>
>> props->stattargets = NIL;
>> ...
>> props->stattargets = GetIndexStatTargets(leafIndexOid);
>>
>
> Agreed and fixed in v15.
>
> BTW, I have run a check-world test with v15.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
> <v15-0001-Preserve-index-per-column-statistics-targets-acr.patch><v15-0002-Preserve-leaf-partition-index-properties-across-.patch>

Rebased to v16 due to a conflict in typedefs.list introduced by 7612ae8ed8c941985d9cbf60a0674c48f5b0abe5.

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