cache lookup failed for constraint when alter table referred by partition table

Started by Rajkumar Raghuwanshiover 7 years ago8 messages
#1Rajkumar Raghuwanshi
rajkumar.raghuwanshi@enterprisedb.com

Hi,

I am getting cache lookup failed for constraint error on master and 11beta3
with below test case.

[edb@localhost bin]$ ./psql postgres
psql (11beta3)
Type "help" for help.

postgres=# CREATE TABLE non_part (a INT,PRIMARY KEY(a));
CREATE TABLE
postgres=# CREATE TABLE part (a INT REFERENCES non_part(a)) PARTITION BY
RANGE(a);
CREATE TABLE
postgres=# CREATE TABLE part_p1 PARTITION OF part FOR VALUES FROM
(MINVALUE) TO (MAXVALUE);
CREATE TABLE
postgres=# ALTER TABLE non_part ALTER COLUMN a TYPE bigint;
*ERROR: cache lookup failed for constraint 16398*

Thanks & Regards,
Rajkumar Raghuwanshi
QMG, EnterpriseDB Corporation

#2Justin Pryzby
pryzby@telsasoft.com
In reply to: Rajkumar Raghuwanshi (#1)
Re: cache lookup failed for constraint when alter table referred by partition table

Adding Alvaro

On Fri, Sep 07, 2018 at 04:02:13PM +0530, Rajkumar Raghuwanshi wrote:

postgres=# CREATE TABLE non_part (a INT,PRIMARY KEY(a));
postgres=# CREATE TABLE part (a INT REFERENCES non_part(a)) PARTITION BY RANGE(a);
postgres=# CREATE TABLE part_p1 PARTITION OF part FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
postgres=# ALTER TABLE non_part ALTER COLUMN a TYPE bigint;
*ERROR: cache lookup failed for constraint 16398*

I want to suggest adding to open items.
https://wiki.postgresql.org/index.php?title=PostgreSQL_11_Open_Items

..since it's documented as an "Major enhancement" in PG11:
https://www.postgresql.org/docs/11/static/release-11.html

Justin

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Justin Pryzby (#2)
Re: cache lookup failed for constraint when alter table referred by partition table

On 2018-Sep-10, Justin Pryzby wrote:

Adding Alvaro

On Fri, Sep 07, 2018 at 04:02:13PM +0530, Rajkumar Raghuwanshi wrote:

postgres=# CREATE TABLE non_part (a INT,PRIMARY KEY(a));
postgres=# CREATE TABLE part (a INT REFERENCES non_part(a)) PARTITION BY RANGE(a);
postgres=# CREATE TABLE part_p1 PARTITION OF part FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
postgres=# ALTER TABLE non_part ALTER COLUMN a TYPE bigint;
*ERROR: cache lookup failed for constraint 16398*

I want to suggest adding to open items.
https://wiki.postgresql.org/index.php?title=PostgreSQL_11_Open_Items

Thanks, looking.

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

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#3)
Re: cache lookup failed for constraint when alter table referred by partition table

On 2018-Sep-10, Alvaro Herrera wrote:

On 2018-Sep-10, Justin Pryzby wrote:

Adding Alvaro

On Fri, Sep 07, 2018 at 04:02:13PM +0530, Rajkumar Raghuwanshi wrote:

postgres=# CREATE TABLE non_part (a INT,PRIMARY KEY(a));
postgres=# CREATE TABLE part (a INT REFERENCES non_part(a)) PARTITION BY RANGE(a);
postgres=# CREATE TABLE part_p1 PARTITION OF part FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
postgres=# ALTER TABLE non_part ALTER COLUMN a TYPE bigint;
*ERROR: cache lookup failed for constraint 16398*

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. Not sure what the fix is
yet, but I'll look into it later today.

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

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#4)
1 attachment(s)
Re: cache lookup failed for constraint when alter table referred by partition table

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

Attachments:

0001-fix-ALTER-TYPE.patchtext/plain; charset=us-asciiDownload
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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#5)
Re: cache lookup failed for constraint when alter table referred by partition table

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

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.

Looks reasonable as far as it goes. Given that we no longer require
any of this:

- * 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.

can we make any simplifications in earlier steps? At the very least,
look for comments related to this assumption.

regards, tom lane

#7Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#6)
1 attachment(s)
Re: cache lookup failed for constraint when alter table referred by partition table

On 2018-Sep-13, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

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.

Looks reasonable as far as it goes. Given that we no longer require
any of this:

- * 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.

can we make any simplifications in earlier steps? At the very least,
look for comments related to this assumption.

Ah, I had looked, but not hard enough. In this new version I removed
some code in ATExecAlterColumnType that's now irrelevant. I tested this
by changing both lappend calls to lcons in that function; seems to
behave the same. (Also added more constraints to the test case.)

Another thing I found I can change is to move the add_object_address()
calls to the other loops scanning the same lists, so that we don't have
to walk each the two lists twice.

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

Attachments:

v2-0001-fix-ALTER-TYPE.patchtext/plain; charset=us-asciiDownload
From 9a9fe65eeb0bc3074793474b9d85b10c3e260e78 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 v2] fix ALTER TYPE

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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e96512e051..03c0b739b3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9527,33 +9527,12 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 				{
 					char	   *defstring = pg_get_constraintdef_command(foundObject.objectId);
 
-					/*
-					 * Put NORMAL dependencies at the front of the list and
-					 * AUTO dependencies at the back.  This makes sure that
-					 * foreign-key constraints depending on this column will
-					 * be dropped before unique or primary-key constraints of
-					 * the column; which we must have because the FK
-					 * constraints depend on the indexes belonging to the
-					 * unique constraints.
-					 */
-					if (foundDep->deptype == DEPENDENCY_NORMAL)
-					{
-						tab->changedConstraintOids =
-							lcons_oid(foundObject.objectId,
-									  tab->changedConstraintOids);
-						tab->changedConstraintDefs =
-							lcons(defstring,
-								  tab->changedConstraintDefs);
-					}
-					else
-					{
-						tab->changedConstraintOids =
-							lappend_oid(tab->changedConstraintOids,
-										foundObject.objectId);
-						tab->changedConstraintDefs =
-							lappend(tab->changedConstraintDefs,
-									defstring);
-					}
+					tab->changedConstraintOids =
+						lappend_oid(tab->changedConstraintOids,
+									foundObject.objectId);
+					tab->changedConstraintDefs =
+						lappend(tab->changedConstraintDefs,
+								defstring);
 				}
 				break;
 
@@ -9893,10 +9872,18 @@ static void
 ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
 {
 	ObjectAddress obj;
+	ObjectAddresses *objects;
 	ListCell   *def_item;
 	ListCell   *oid_item;
 
 	/*
+	 * Collect all the constraints and indexes to drop so we can process them
+	 * in a single call.  That way we don't have to worry about dependencies
+	 * among them.
+	 */
+	objects = new_object_addresses();
+
+	/*
 	 * Re-parse the index and constraint definitions, and attach them to the
 	 * appropriate work queue entries.  We do this before dropping because in
 	 * the case of a FOREIGN KEY constraint, we might not yet have exclusive
@@ -9937,6 +9924,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
 		conislocal = con->conislocal;
 		ReleaseSysCache(tup);
 
+		ObjectAddressSet(obj, ConstraintRelationId, lfirst_oid(oid_item));
+		add_exact_object_address(&obj, objects);
+
 		/*
 		 * If the constraint is inherited (only), we don't want to inject a
 		 * new definition here; it'll get recreated when ATAddCheckConstraint
@@ -9960,31 +9950,18 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
 		ATPostAlterTypeParse(oldId, relid, InvalidOid,
 							 (char *) lfirst(def_item),
 							 wqueue, lockmode, tab->rewrite);
+
+		ObjectAddressSet(obj, RelationRelationId, lfirst_oid(oid_item));
+		add_exact_object_address(&obj, objects);
 	}
 
 	/*
-	 * 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.
+	 * It should be okay to use DROP_RESTRICT here, since nothing else should
+	 * be depending on these objects.
 	 */
-	foreach(oid_item, tab->changedConstraintOids)
-	{
-		obj.classId = ConstraintRelationId;
-		obj.objectId = lfirst_oid(oid_item);
-		obj.objectSubId = 0;
-		performDeletion(&obj, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
-	}
+	performMultipleDeletions(objects, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
 
-	foreach(oid_item, tab->changedIndexOids)
-	{
-		obj.classId = RelationRelationId;
-		obj.objectId = lfirst_oid(oid_item);
-		obj.objectSubId = 0;
-		performDeletion(&obj, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
-	}
+	free_object_addresses(objects);
 
 	/*
 	 * The objects will get recreated during subsequent passes over the work
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index b90c4926e2..fc3bbe4deb 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), CHECK (a > 0));
+CREATE TABLE fk_partitioned_fk (a INT REFERENCES fk_notpartitioned_pk(a) PRIMARY KEY) 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..d2cecdf4eb 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), CHECK (a > 0));
+CREATE TABLE fk_partitioned_fk (a INT REFERENCES fk_notpartitioned_pk(a) PRIMARY KEY) 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

#8Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#7)
Re: cache lookup failed for constraint when alter table referred by partition table

Thanks Rajkumar, Tom, Justin -- pushed fix.

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