Обсуждение: cache lookup failed for constraint when alter table referred bypartition 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

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

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

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

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

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

Thanks Rajkumar, Tom, Justin -- pushed fix.

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