On 2019-03-29 16:10, Shinoda, Noriyoshi (PN Japan A&PS Delivery) wrote:
> postgres=> CREATE TABLE part1(c1 INT) PARTITION BY RANGE(c1);
> CREATE TABLE
> postgres=> CREATE TABLE part1v1 PARTITION OF part1 FOR VALUES FROM (0) TO 
> (100);
> CREATE TABLE
> postgres=> CREATE INDEX idx1_part1 ON part1(c1);
> CREATE INDEX
> postgres=> REINDEX TABLE CONCURRENTLY part1v1;
> ERROR:  cannot drop index part1v1_c1_idx_ccold because index idx1_part1 
> requires it
> HINT:  You can drop index idx1_part1 instead.

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.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 66a5ce802ede0bd28a6f4881e063ac92a35c0c8c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Sat, 30 Mar 2019 09:37:19 +0100
Subject: [PATCH] Fix REINDEX CONCURRENTLY of partitions

When swapping the old and new index, we not only need to change
dependencies referencing the index, but also dependencies of the
index referencing something else.  The previous code did this only
specifically for a constraint, but we also need to do this for
partitioned indexes.  So instead write a generic function that does it
for all dependencies.
---
 src/backend/catalog/index.c                | 24 +---------
 src/backend/catalog/pg_depend.c            | 56 ++++++++++++++++++++++
 src/include/catalog/dependency.h           |  3 ++
 src/test/regress/expected/create_index.out |  6 +++
 src/test/regress/sql/create_index.sql      |  6 +++
 5 files changed, 73 insertions(+), 22 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 0d9d405c54..77e2088034 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1555,29 +1555,9 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, 
const char *oldName)
        }
 
        /*
-        * Move all dependencies on the old index to the new one
+        * Move all dependencies of and on the old index to the new one
         */
-
-       if (OidIsValid(indexConstraintOid))
-       {
-               ObjectAddress myself,
-                                       referenced;
-
-               /* Change to having the new index depend on the constraint */
-               deleteDependencyRecordsForClass(RelationRelationId, oldIndexId,
-                                                                               
ConstraintRelationId, DEPENDENCY_INTERNAL);
-
-               myself.classId = RelationRelationId;
-               myself.objectId = newIndexId;
-               myself.objectSubId = 0;
-
-               referenced.classId = ConstraintRelationId;
-               referenced.objectId = indexConstraintOid;
-               referenced.objectSubId = 0;
-
-               recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL);
-       }
-
+       changeDependenciesOf(RelationRelationId, oldIndexId, newIndexId);
        changeDependenciesOn(RelationRelationId, oldIndexId, newIndexId);
 
        /*
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index d63bf5e56d..f7caedcc02 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -395,6 +395,62 @@ changeDependencyFor(Oid classId, Oid objectId,
        return count;
 }
 
+/*
+ * Adjust all dependency records to come from a different object of the same 
type
+ *
+ * classId/oldObjectId specify the old referencing object.
+ * newObjectId is the new referencing object (must be of class classId).
+ *
+ * Returns the number of records updated.
+ */
+long
+changeDependenciesOf(Oid classId, Oid oldObjectId,
+                                        Oid newObjectId)
+{
+       long            count = 0;
+       Relation        depRel;
+       ScanKeyData key[2];
+       SysScanDesc scan;
+       HeapTuple       tup;
+
+       depRel = table_open(DependRelationId, RowExclusiveLock);
+
+       ScanKeyInit(&key[0],
+                               Anum_pg_depend_classid,
+                               BTEqualStrategyNumber, F_OIDEQ,
+                               ObjectIdGetDatum(classId));
+       ScanKeyInit(&key[1],
+                               Anum_pg_depend_objid,
+                               BTEqualStrategyNumber, F_OIDEQ,
+                               ObjectIdGetDatum(oldObjectId));
+
+       scan = systable_beginscan(depRel, DependDependerIndexId, true,
+                                                         NULL, 2, key);
+
+       while (HeapTupleIsValid((tup = systable_getnext(scan))))
+       {
+               Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup);
+
+               /* make a modifiable copy */
+               tup = heap_copytuple(tup);
+               depform = (Form_pg_depend) GETSTRUCT(tup);
+
+               depform->objid = newObjectId;
+
+               CatalogTupleUpdate(depRel, &tup->t_self, tup);
+
+               heap_freetuple(tup);
+
+               count++;
+       }
+
+       systable_endscan(scan);
+
+       table_close(depRel, RowExclusiveLock);
+
+       return count;
+}
+
 /*
  * Adjust all dependency records to point to a different object of the same 
type
  *
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 4f9dde9df9..57545b70d8 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -199,6 +199,9 @@ extern long changeDependencyFor(Oid classId, Oid objectId,
                                        Oid refClassId, Oid oldRefObjectId,
                                        Oid newRefObjectId);
 
+extern long changeDependenciesOf(Oid classId, Oid oldObjectId,
+                                                                Oid 
newObjectId);
+
 extern long changeDependenciesOn(Oid refClassId, Oid oldRefObjectId,
                                                                 Oid 
newRefObjectId);
 
diff --git a/src/test/regress/expected/create_index.out 
b/src/test/regress/expected/create_index.out
index 6b77d25deb..61c7a3a67f 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -3312,6 +3312,12 @@ SELECT obj_description('testcomment_idx1'::regclass, 
'pg_class');
 (1 row)
 
 DROP TABLE testcomment;
+-- partitions
+CREATE TABLE concur_reindex_part1 (c1 int) PARTITION BY RANGE (c1);
+CREATE TABLE concur_reindex_part1v1 PARTITION OF concur_reindex_part1 FOR 
VALUES FROM (0) TO (100);
+CREATE INDEX concur_reindex_idx1_part1 ON concur_reindex_part1 (c1);
+REINDEX TABLE CONCURRENTLY concur_reindex_part1v1;
+DROP TABLE concur_reindex_part1;
 -- Check errors
 -- Cannot run inside a transaction block
 BEGIN;
diff --git a/src/test/regress/sql/create_index.sql 
b/src/test/regress/sql/create_index.sql
index 9ff2dc68ff..559a6e5cb8 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1214,6 +1214,12 @@ CREATE INDEX testcomment_idx1 ON testcomment (i);
 REINDEX TABLE CONCURRENTLY testcomment ;
 SELECT obj_description('testcomment_idx1'::regclass, 'pg_class');
 DROP TABLE testcomment;
+-- partitions
+CREATE TABLE concur_reindex_part1 (c1 int) PARTITION BY RANGE (c1);
+CREATE TABLE concur_reindex_part1v1 PARTITION OF concur_reindex_part1 FOR 
VALUES FROM (0) TO (100);
+CREATE INDEX concur_reindex_idx1_part1 ON concur_reindex_part1 (c1);
+REINDEX TABLE CONCURRENTLY concur_reindex_part1v1;
+DROP TABLE concur_reindex_part1;
 
 -- Check errors
 -- Cannot run inside a transaction block
-- 
2.21.0

Reply via email to