Check that partitions are not in use when dropping constraints

If the user creates a deferred constraint in a partition, and in a
transaction they cause the constraint's trigger execution to be deferred
until commit time *and* drop the constraint, then when commit time comes
the queued trigger will fail to run because the trigger object will have
been dropped.

This is explained because when a constraint gets dropped in a
partitioned table, the recursion to drop the ones in partitions is done
by the dependency mechanism, not by ALTER TABLE traversing the recursion
tree as in all other cases.  In the non-partitioned case, this problem
is avoided by checking that the table is not "in use" by alter-table;
other alter-table subcommands that recurse to partitions do that check
for each partition.  But the dependency mechanism doesn't have a way to
do that.  Fix the problem by applying the same check to all partitions
during ALTER TABLE's "prep" phase, which correctly raises the necessary
error.

Reported-by: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>
Discussion: https://postgr.es/m/CAKcux6nZiO9-eEpr1ZD84bT1mBoVmeZkfont8iSpcmYrjhGWgA@mail.gmail.com

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2ceda4f0a16547a4dda113cd683cef7d8621a84b..836e0845c0d5c780fcbd3928130935236169966b 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -353,6 +353,7 @@ static void ATSimplePermissions(Relation rel, int allowed_targets);
 static void ATWrongRelkindError(Relation rel, int allowed_targets);
 static void ATSimpleRecursion(List **wqueue, Relation rel,
                                  AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode);
+static void ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode);
 static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
                                          LOCKMODE lockmode);
 static List *find_typed_table_dependencies(Oid typeOid, const char *typeName,
@@ -3269,8 +3270,7 @@ CheckTableNotInUse(Relation rel, const char *stmt)
                ereport(ERROR,
                                (errcode(ERRCODE_OBJECT_IN_USE),
                /* translator: first %s is a SQL command, eg ALTER TABLE */
-                                errmsg("cannot %s \"%s\" because "
-                                               "it is being used by active queries in this session",
+                                errmsg("cannot %s \"%s\" because it is being used by active queries in this session",
                                                stmt, RelationGetRelationName(rel))));
 
        if (rel->rd_rel->relkind != RELKIND_INDEX &&
@@ -3279,8 +3279,7 @@ CheckTableNotInUse(Relation rel, const char *stmt)
                ereport(ERROR,
                                (errcode(ERRCODE_OBJECT_IN_USE),
                /* translator: first %s is a SQL command, eg ALTER TABLE */
-                                errmsg("cannot %s \"%s\" because "
-                                               "it has pending trigger events",
+                                errmsg("cannot %s \"%s\" because it has pending trigger events",
                                                stmt, RelationGetRelationName(rel))));
 }
 
@@ -3746,16 +3745,19 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        break;
                case AT_AddIdentity:
                        ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+                       /* This command never recurses */
                        pass = AT_PASS_ADD_CONSTR;
                        break;
-               case AT_DropIdentity:
-                       ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
-                       pass = AT_PASS_DROP;
-                       break;
                case AT_SetIdentity:
                        ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+                       /* This command never recurses */
                        pass = AT_PASS_COL_ATTRS;
                        break;
+               case AT_DropIdentity:
+                       ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+                       /* This command never recurses */
+                       pass = AT_PASS_DROP;
+                       break;
                case AT_DropNotNull:    /* ALTER COLUMN DROP NOT NULL */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        ATPrepDropNotNull(rel, recurse, recursing);
@@ -3817,7 +3819,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        break;
                case AT_DropConstraint: /* DROP CONSTRAINT */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
-                       /* Recursion occurs during execution phase */
+                       ATCheckPartitionsNotInUse(rel, lockmode);
+                       /* Other recursion occurs during execution phase */
                        /* No command-specific prep needed except saving recurse flag */
                        if (recurse)
                                cmd->subtype = AT_DropConstraintRecurse;
@@ -5085,8 +5088,9 @@ ATSimpleRecursion(List **wqueue, Relation rel,
                                  AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode)
 {
        /*
-        * Propagate to children if desired.  Only plain tables and foreign tables
-        * have children, so no need to search for other relkinds.
+        * Propagate to children if desired.  Only plain tables, foreign tables
+        * and partitioned tables have children, so no need to search for other
+        * relkinds.
         */
        if (recurse &&
                (rel->rd_rel->relkind == RELKIND_RELATION ||
@@ -5120,6 +5124,36 @@ ATSimpleRecursion(List **wqueue, Relation rel,
        }
 }
 
+/*
+ * Obtain list of partitions of the given table, locking them all at the given
+ * lockmode and ensuring that they all pass CheckTableNotInUse.
+ *
+ * This function is a no-op if the given relation is not a partitioned table;
+ * in particular, nothing is done if it's a legacy inheritance parent.
+ */
+static void
+ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode)
+{
+       if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+       {
+               List       *inh;
+               ListCell   *cell;
+
+               inh = find_all_inheritors(RelationGetRelid(rel), lockmode, NULL);
+               /* first element is the parent rel; must ignore it */
+               for_each_cell(cell, lnext(list_head(inh)))
+               {
+                       Relation        childrel;
+
+                       /* find_all_inheritors already got lock */
+                       childrel = heap_open(lfirst_oid(cell), NoLock);
+                       CheckTableNotInUse(childrel, "ALTER TABLE");
+                       heap_close(childrel, NoLock);
+               }
+               list_free(inh);
+       }
+}
+
 /*
  * ATTypedTableRecursion
  *

diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index d283ca661a726dfa6a16f147a0e38205e2c7e0b5..754a9d5ae1b84a26d5f0e0852f6a3c05e021de8f 100644 (file)
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1926,7 +1926,18 @@ alter table fkpart2.fk_part detach partition fkpart2.fk_part_1;
 alter table fkpart2.fk_part_1 drop constraint fkey;    -- ok
 alter table fkpart2.fk_part_1_1 drop constraint my_fkey;       -- doesn't exist
 ERROR:  constraint "my_fkey" of relation "fk_part_1_1" does not exist
+-- ensure we check partitions are "not used" when dropping constraints
+CREATE SCHEMA fkpart8
+  CREATE TABLE tbl1(f1 int PRIMARY KEY)
+  CREATE TABLE tbl2(f1 int REFERENCES tbl1 DEFERRABLE INITIALLY DEFERRED) PARTITION BY RANGE(f1)
+  CREATE TABLE tbl2_p1 PARTITION OF tbl2 FOR VALUES FROM (minvalue) TO (maxvalue);
+INSERT INTO fkpart8.tbl1 VALUES(1);
+BEGIN;
+INSERT INTO fkpart8.tbl2 VALUES(1);
+ALTER TABLE fkpart8.tbl2 DROP CONSTRAINT tbl2_f1_fkey;
+ERROR:  cannot ALTER TABLE "tbl2_p1" because it has pending trigger events
+COMMIT;
 \set VERBOSITY terse   \\ -- suppress cascade details
-drop schema fkpart0, fkpart1, fkpart2 cascade;
-NOTICE:  drop cascades to 8 other objects
+drop schema fkpart0, fkpart1, fkpart2, fkpart8 cascade;
+NOTICE:  drop cascades to 10 other objects
 \set VERBOSITY default

diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 2dcbfe4cf8c10a3487f5b8f1170167cd8e4fc358..3f8e9b83d345e12e934b640d42af3c6ba8184a7d 100644 (file)
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1380,6 +1380,17 @@ alter table fkpart2.fk_part detach partition fkpart2.fk_part_1;
 alter table fkpart2.fk_part_1 drop constraint fkey;    -- ok
 alter table fkpart2.fk_part_1_1 drop constraint my_fkey;       -- doesn't exist
 
+-- ensure we check partitions are "not used" when dropping constraints
+CREATE SCHEMA fkpart8
+  CREATE TABLE tbl1(f1 int PRIMARY KEY)
+  CREATE TABLE tbl2(f1 int REFERENCES tbl1 DEFERRABLE INITIALLY DEFERRED) PARTITION BY RANGE(f1)
+  CREATE TABLE tbl2_p1 PARTITION OF tbl2 FOR VALUES FROM (minvalue) TO (maxvalue);
+INSERT INTO fkpart8.tbl1 VALUES(1);
+BEGIN;
+INSERT INTO fkpart8.tbl2 VALUES(1);
+ALTER TABLE fkpart8.tbl2 DROP CONSTRAINT tbl2_f1_fkey;
+COMMIT;
+
 \set VERBOSITY terse   \\ -- suppress cascade details
-drop schema fkpart0, fkpart1, fkpart2 cascade;
+drop schema fkpart0, fkpart1, fkpart2, fkpart8 cascade;
 \set VERBOSITY default