Fix partitioned index attachment

When an existing index in a partition is attached to a new index on
its parent, we forgot to set the "relispartition" flag correctly, which
meant that it was not possible to find the index in various operations,
such as adding a foreign key constraint that references that partitioned
table.  One of four places that was assigning the parent index was
forgetting to do that, so fix by shifting responsibility of updating the
flag to the routine that changes the parent.

Author: Amit Langote, Álvaro Herrera
Reported-by: Hubert "depesz" Lubaczewski
Discussion: https://postgr.es/m/CA+HiwqHMsRtRYRWYTWavKJ8x14AFsv7bmAV46mYwnfD3vy8goQ@mail.gmail.com

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 3599c0d8ce07ea44acdfeb3ffd3914525f564ff1..ed19f77ba0ae4f1fad97a6398fa8e898d9011cf9 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -89,6 +89,7 @@ static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
                                                                Oid relId, Oid oldRelId, void *arg);
 static bool ReindexRelationConcurrently(Oid relationOid, int options);
 static void ReindexPartitionedIndex(Relation parentIdx);
+static void update_relispartition(Oid relationId, bool newval);
 
 /*
  * CheckIndexCompatible
@@ -3388,6 +3389,9 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
        if (OidIsValid(parentOid))
                SetRelationHasSubclass(parentOid, true);
 
+       /* set relispartition correctly on the partition */
+       update_relispartition(partRelid, OidIsValid(parentOid));
+
        if (fix_dependencies)
        {
                /*
@@ -3424,3 +3428,23 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
                CommandCounterIncrement();
        }
 }
+
+/*
+ * Subroutine of IndexSetParentIndex to update the relispartition flag of the
+ * given index to the given value.
+ */
+static void
+update_relispartition(Oid relationId, bool newval)
+{
+       HeapTuple       tup;
+       Relation        classRel;
+
+       classRel = table_open(RelationRelationId, RowExclusiveLock);
+       tup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relationId));
+       Assert(((Form_pg_class) GETSTRUCT(tup))->relispartition != newval);
+       ((Form_pg_class) GETSTRUCT(tup))->relispartition = newval;
+       CatalogTupleUpdate(classRel, &tup->t_self, tup);
+       heap_freetuple(tup);
+
+       table_close(classRel, RowExclusiveLock);
+}

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 66122afb077dcd542a90e756467bda302c98b2cf..b2f8189fa161732ac85718742a010a13d5f42818 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -525,8 +525,6 @@ static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation rel,
 static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl);
 static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx,
                                          Relation partitionTbl);
-static void update_relispartition(Relation classRel, Oid relationId,
-                                         bool newval);
 static List *GetParentedForeignKeyRefs(Relation partition);
 static void ATDetachCheckNoForeignKeyRefs(Relation partition);
 
@@ -15714,7 +15712,6 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
                                if (OidIsValid(constraintOid))
                                        ConstraintSetParentConstraint(cldConstrOid, constraintOid,
                                                                                                  RelationGetRelid(attachrel));
-                               update_relispartition(NULL, cldIdxId, true);
                                found = true;
 
                                CommandCounterIncrement();
@@ -15970,7 +15967,6 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 
                idx = index_open(idxid, AccessExclusiveLock);
                IndexSetParentIndex(idx, InvalidOid);
-               update_relispartition(classRel, idxid, false);
 
                /* If there's a constraint associated with the index, detach it too */
                constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
@@ -16268,7 +16264,6 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
                if (OidIsValid(constraintOid))
                        ConstraintSetParentConstraint(cldConstrId, constraintOid,
                                                                                  RelationGetRelid(partTbl));
-               update_relispartition(NULL, partIdxId, true);
 
                pfree(attmap);
 
@@ -16401,38 +16396,6 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl)
        }
 }
 
-/*
- * Update the relispartition flag of the given relation to the given value.
- *
- * classRel is the pg_class relation, already open and suitably locked.
- * It can be passed as NULL, in which case it's opened and closed locally.
- */
-static void
-update_relispartition(Relation classRel, Oid relationId, bool newval)
-{
-       HeapTuple       tup;
-       HeapTuple       newtup;
-       Form_pg_class classForm;
-       bool            opened = false;
-
-       if (classRel == NULL)
-       {
-               classRel = table_open(RelationRelationId, RowExclusiveLock);
-               opened = true;
-       }
-
-       tup = SearchSysCache1(RELOID, ObjectIdGetDatum(relationId));
-       newtup = heap_copytuple(tup);
-       classForm = (Form_pg_class) GETSTRUCT(newtup);
-       classForm->relispartition = newval;
-       CatalogTupleUpdate(classRel, &tup->t_self, newtup);
-       heap_freetuple(newtup);
-       ReleaseSysCache(tup);
-
-       if (opened)
-               table_close(classRel, RowExclusiveLock);
-}
-
 /*
  * Return an OID list of constraints that reference the given relation
  * that are marked as having a parent constraints.

diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 3dd33f6aa16c1c88cbc6c7433b7b1f649c59cadd..58f6058e70cff20576863d6f17f0ae668c0a712a 100644 (file)
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2367,3 +2367,15 @@ SELECT tableoid::regclass, * FROM fk;
 (2 rows)
 
 DROP TABLE fk;
+-- test for reported bug: relispartition not set
+-- https://postgr.es/m/CA+HiwqHMsRtRYRWYTWavKJ8x14AFsv7bmAV46mYwnfD3vy8goQ@mail.gmail.com
+CREATE SCHEMA fkpart7
+  CREATE TABLE pkpart (a int) PARTITION BY LIST (a)
+  CREATE TABLE pkpart1 PARTITION OF pkpart FOR VALUES IN (1);
+ALTER TABLE fkpart7.pkpart1 ADD PRIMARY KEY (a);
+ALTER TABLE fkpart7.pkpart ADD PRIMARY KEY (a);
+CREATE TABLE fkpart7.fk (a int REFERENCES fkpart7.pkpart);
+DROP SCHEMA fkpart7 CASCADE;
+NOTICE:  drop cascades to 2 other objects
+DETAIL:  drop cascades to table fkpart7.pkpart
+drop cascades to table fkpart7.fk

diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 672fce317f1dc7273ed6f63e98c7f277d58d6e29..9c2828571729df0e5f3cca98f05c8d2418e55f1e 100644 (file)
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1657,3 +1657,13 @@ DELETE FROM pk WHERE a = 20;
 UPDATE pk SET a = 90 WHERE a = 30;
 SELECT tableoid::regclass, * FROM fk;
 DROP TABLE fk;
+
+-- test for reported bug: relispartition not set
+-- https://postgr.es/m/CA+HiwqHMsRtRYRWYTWavKJ8x14AFsv7bmAV46mYwnfD3vy8goQ@mail.gmail.com
+CREATE SCHEMA fkpart7
+  CREATE TABLE pkpart (a int) PARTITION BY LIST (a)
+  CREATE TABLE pkpart1 PARTITION OF pkpart FOR VALUES IN (1);
+ALTER TABLE fkpart7.pkpart1 ADD PRIMARY KEY (a);
+ALTER TABLE fkpart7.pkpart ADD PRIMARY KEY (a);
+CREATE TABLE fkpart7.fk (a int REFERENCES fkpart7.pkpart);
+DROP SCHEMA fkpart7 CASCADE;