Re: REINDEX CONCURRENTLY 2.0

On Mon, Apr 01, 2019 at 03:43:43PM +0900, Michael Paquier wrote:
> And I would have expected concur_reindex_part1v1_c1_idx to still be
> part of the partition tree. I think that the issue is in
> index_concurrently_create_copy() where we create the new index with
> index_create() without setting parentIndexRelid, causing the
> dependency to be lost. This parameter ought to be set to the OID of
> the parent index so I think that we need to look at the ancestors of
> the index if relispartition is set, and use get_partition_ancestors()
> for that purpose.

And here is the patch to address this issue. It happens that a bit
more than the dependency switch was lacking here:
- At swap time, we need to have the new index definition track
relispartition from the old index.
- Again at swap time, the inheritance link needs to be updated between
the old/new index and its parent when reindexing a partition index.

Tracking the OID of the parent via index_concurrently_create_copy() is
not a bright idea as we would finish with the impossibility to drop
invalid indexes if the REINDEX CONCURRENTLY failed in the middle (just
added some manual elog(ERROR) to test that). I have added a comment
before making the index duplica. I have also expanded the regression
tests so as we have more coverage for all that, finishing with the
attached which keeps partition trees consistent across the operations.
Thoughts?
--
Michael