Date: Thu, 13 Sep 2018 13:35:04 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: cache lookup failed for constraint when alter table referred by
 partition table
Message-ID: <20180913163504.ecvozcnighb6cbxm@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="rx6whfazzpcg2bu2"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180910143637.3xscglivuau6mpz5@alvherre.pgsql>
User-Agent: NeoMutt/20170306-137-4415bd-dirty (1.8.0)
Precedence: bulk


--rx6whfazzpcg2bu2
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2018-Sep-10, Alvaro Herrera wrote:

> ATPostAlterTypeCleanup is trying to search the original constraint by
> OID in order to drop it, but it's not there -- I suppose it has already
> been dropped by recursion in a previous step.

That's the problem all right.  The solution is to drop all
index/constraint objects together in one performMultipleDeletions()
instead of performDeletion() one by one, as in the attached patch.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--rx6whfazzpcg2bu2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="0001-fix-ALTER-TYPE.patch"

From 4e1ec5fcb396e3c71fc399c986d9bbd9610d7849 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 13 Sep 2018 13:26:18 -0300
Subject: [PATCH] fix ALTER TYPE

---
 src/backend/commands/tablecmds.c          | 28 ++++++++++++++--------------
 src/test/regress/expected/foreign_key.out | 12 ++++++++++++
 src/test/regress/sql/foreign_key.sql      | 11 +++++++++++
 3 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e96512e051..f8c5d71ccf 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9893,6 +9893,7 @@ static void
 ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
 {
 	ObjectAddress obj;
+	ObjectAddresses *objects;
 	ListCell   *def_item;
 	ListCell   *oid_item;
 
@@ -9963,29 +9964,28 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
 	}
 
 	/*
-	 * Now we can drop the existing constraints and indexes --- constraints
-	 * first, since some of them might depend on the indexes.  In fact, we
-	 * have to delete FOREIGN KEY constraints before UNIQUE constraints, but
-	 * we already ordered the constraint list to ensure that would happen. It
-	 * should be okay to use DROP_RESTRICT here, since nothing else should be
-	 * depending on these objects.
+	 * Now we can drop the existing constraints and indexes. Do them all in a
+	 * single call, so that we don't have to worry about dependencies among
+	 * them.  It should be okay to use DROP_RESTRICT here, since nothing else
+	 * should be depending on these objects.
 	 */
+	objects = new_object_addresses();
 	foreach(oid_item, tab->changedConstraintOids)
 	{
-		obj.classId = ConstraintRelationId;
-		obj.objectId = lfirst_oid(oid_item);
-		obj.objectSubId = 0;
-		performDeletion(&obj, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+		ObjectAddressSet(obj, ConstraintRelationId, lfirst_oid(oid_item));
+		add_exact_object_address(&obj, objects);
 	}
 
 	foreach(oid_item, tab->changedIndexOids)
 	{
-		obj.classId = RelationRelationId;
-		obj.objectId = lfirst_oid(oid_item);
-		obj.objectSubId = 0;
-		performDeletion(&obj, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+		ObjectAddressSet(obj, RelationRelationId, lfirst_oid(oid_item));
+		add_exact_object_address(&obj, objects);
 	}
 
+	performMultipleDeletions(objects, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+
+	free_object_addresses(objects);
+
 	/*
 	 * The objects will get recreated during subsequent passes over the work
 	 * queue.
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index b90c4926e2..1854867381 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1518,6 +1518,18 @@ DETAIL:  Key (a, b)=(2500, 2502) is still referenced from table "fk_partitioned_
 ALTER TABLE fk_partitioned_fk DROP CONSTRAINT fk_partitioned_fk_a_fkey;
 -- done.
 DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk;
+-- Altering a type referenced by a foreign key needs to drop/recreate the FK.
+-- Ensure that works.
+CREATE TABLE fk_notpartitioned_pk (a INT, PRIMARY KEY(a));
+CREATE TABLE fk_partitioned_fk (a INT REFERENCES fk_notpartitioned_pk(a)) PARTITION BY RANGE(a);
+CREATE TABLE fk_partitioned_fk_1 PARTITION OF fk_partitioned_fk FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
+INSERT INTO fk_notpartitioned_pk VALUES (1);
+INSERT INTO fk_partitioned_fk VALUES (1);
+ALTER TABLE fk_notpartitioned_pk ALTER COLUMN a TYPE bigint;
+DELETE FROM fk_notpartitioned_pk WHERE a = 1;
+ERROR:  update or delete on table "fk_notpartitioned_pk" violates foreign key constraint "fk_partitioned_fk_a_fkey" on table "fk_partitioned_fk"
+DETAIL:  Key (a)=(1) is still referenced from table "fk_partitioned_fk".
+DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk;
 -- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE
 -- actions
 CREATE TABLE fk_notpartitioned_pk (a int, b int, primary key (a, b));
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index f3e7058583..825c76335b 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1139,6 +1139,17 @@ ALTER TABLE fk_partitioned_fk DROP CONSTRAINT fk_partitioned_fk_a_fkey;
 -- done.
 DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk;
 
+-- Altering a type referenced by a foreign key needs to drop/recreate the FK.
+-- Ensure that works.
+CREATE TABLE fk_notpartitioned_pk (a INT, PRIMARY KEY(a));
+CREATE TABLE fk_partitioned_fk (a INT REFERENCES fk_notpartitioned_pk(a)) PARTITION BY RANGE(a);
+CREATE TABLE fk_partitioned_fk_1 PARTITION OF fk_partitioned_fk FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
+INSERT INTO fk_notpartitioned_pk VALUES (1);
+INSERT INTO fk_partitioned_fk VALUES (1);
+ALTER TABLE fk_notpartitioned_pk ALTER COLUMN a TYPE bigint;
+DELETE FROM fk_notpartitioned_pk WHERE a = 1;
+DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk;
+
 -- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE
 -- actions
 CREATE TABLE fk_notpartitioned_pk (a int, b int, primary key (a, b));
-- 
2.11.0


--rx6whfazzpcg2bu2--