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)
