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

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