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