Re: BUG #18016: REINDEX TABLE failure

On Sun, Jul 09, 2023 at 12:01:03AM -0700, Gurjeet Singh wrote:
> Preliminary reading of the code indicates that a simple rearrangement
> of the code in reindex_relation() would be sufficient to get the
> desired behaviour. The code towards the bottom in that function,
> protected by `if ((flags & REINDEX_REL_PROCESS_TOAST ...)` needs to be
> moved to just before the `foreach(indexId, indexIds)` loop.

I guess that it should be OK to do that from the point where
reltoastrelid is known, when extracted the parent relation locked with
this ShareLock.

> The only downside I see so far with the proposed change is that the
> toast tables are currently reindexed after table_close() call, but
> after the proposed change they'll be reindexed before that call to
> close_table(). But since close_table() does not release the ShareLock
> on the table that is taken at the beginning of reindex_relation(), I
> don't think we'll losing anything by the proposed rearrangement of
> code.

That should be OK, I assume.  However, if this is improved and
something we want to support in the long-run I guess that a TAP test
may be appropriate.
--
Michael