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

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:

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.

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.

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;
}

3. It's unnecessary to set stattargets = NIL when you'll overwrite it right
after anyway.

props->stattargets = NIL;
...
props->stattargets = GetIndexStatTargets(leafIndexOid);

Best Regards,
Zhijie Hou