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
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 Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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 Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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 Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 4e1ec5fcb396e3c71fc399c986d9bbd9610d7849 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
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/sq
Re: cache lookup failed for constraint when alter table referred by partition table
Alvaro Herrera 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
Re: cache lookup failed for constraint when alter table referred by partition table
On 2018-Sep-13, Tom Lane wrote:
> Alvaro Herrera 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 Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 9a9fe65eeb0bc3074793474b9d85b10c3e260e78 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
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)
{
Objec
Re: cache lookup failed for constraint when alter table referred by partition table
Thanks Rajkumar, Tom, Justin -- pushed fix. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
