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

Hi Chao,

Thanks for the update.

> I have addressed them in v15 expect for the hash-table suggestion. I
tried using an HTAB, but ran into several build failures.

The v15 changes LGTM. I am fine with leaving the HTAB idea aside. This is
not a performance critical path in general, so I do not think it is worth
adding complexity there. After thinking more about it, I can see why it
could be difficult, especially if we need to track hash state outside of
IndexStmt. We can still optimize the lookup by scoping an HTAB inside
DefineIndex(). In that case we would still carry the list in IndexStmt, but
we could make the lookups within DefineIndex() more efficient with a
function-scoped HTAB. But, again, not sure it's worth it.

One other point from yesterday remains. We should invoke
InvokeObjectPostAlterHook() after the CatalogTupleUpdate() in
SetIndexStatTargets(), and InvokeObjectPostAlterHookArg() after the direct
pg_index CatalogTupleUpdate() in DefineIndex() inside the
stmt->idxisclustered || stmt->idxisreplident block.

I think this is the right thing to do because, although we are restoring
existing values, these are being restored onto a newly created index with a
new OID.

--
Sami Imseih
Amazon Web Services (AWS)