Re: REINDEX CONCURRENTLY 2.0

On Sat, Mar 30, 2019 at 11:56:27AM +0100, Peter Eisentraut wrote:
> The attached patch fixes this.  The issue was that we didn't move all
> dependencies from the index (only in the other direction).  Maybe that
> was sufficient when the patch was originally written, before partitioned
> indexes.

Hm.  I don't think that it is quite right either because the new index
is missing from the partition tree after the reindex.  Taking the
example from your patch I see that:
=# CREATE TABLE concur_reindex_part1 (c1 int) PARTITION BY RANGE (c1);
CREATE TABLE
=# CREATE TABLE concur_reindex_part1v1 PARTITION OF
     concur_reindex_part1 FOR VALUES FROM (0) TO (100);
CREATE TABLE
=# SELECT relid, level FROM
   pg_partition_tree('concur_reindex_idx1_part1');
             relid             | level
-------------------------------+-------
 concur_reindex_idx1_part1     |     0
 concur_reindex_part1v1_c1_idx |     1
(2 rows)
=# CREATE INDEX concur_reindex_idx1_part1 ON
     concur_reindex_part1 (c1);
CREATE INDEX
=# REINDEX TABLE CONCURRENTLY concur_reindex_part1v1;
REINDEX
SELECT relid, level FROM
   pg_partition_tree('concur_reindex_idx1_part1');
           relid           | level
---------------------------+-------
 concur_reindex_idx1_part1 |     0
(1 row)

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.
--
Michael