Re: BUG #18970: Atempt to alter type of table column used in row type with check leads to assertion failure - Mailing list pgsql-bugs

From Tom Lane
Subject Re: BUG #18970: Atempt to alter type of table column used in row type with check leads to assertion failure
Date
Msg-id 2537374.1751218360@sss.pgh.pa.us
Whole thread Raw
In response to Re: BUG #18970: Atempt to alter type of table column used in row type with check leads to assertion failure  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-bugs
I wrote:
> jian he <jian.universality@gmail.com> writes:
>> we can change to
>>     if (relid != tab->relid)
>>         LockRelationOid(relid, AccessExclusiveLock);
>> obviously, the comments need to be updated.

> Yeah, I came to the same conclusion after studying it for awhile.

Here's a patch for that.  Working on back-patching it ...

            regards, tom lane

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 991bc946ffc..b8837f26cb4 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15415,9 +15415,12 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
     /*
      * 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
-     * lock on the table the constraint is attached to, and we need to get
-     * that before reparsing/dropping.
+     * the case of a constraint on another table, we might not yet have
+     * exclusive lock on the table the constraint is attached to, and we need
+     * to get that before reparsing/dropping.  (That's possible at least for
+     * FOREIGN KEY, CHECK, and EXCLUSION constraints; in non-FK cases it
+     * requires a dependency on the target table's composite type in the other
+     * table's constraint expressions.)
      *
      * We can't rely on the output of deparsing to tell us which relation to
      * operate on, because concurrent activity might have made the name
@@ -15433,7 +15436,6 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
         Form_pg_constraint con;
         Oid            relid;
         Oid            confrelid;
-        char        contype;
         bool        conislocal;

         tup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(oldId));
@@ -15450,7 +15452,6 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
                 elog(ERROR, "could not identify relation associated with constraint %u", oldId);
         }
         confrelid = con->confrelid;
-        contype = con->contype;
         conislocal = con->conislocal;
         ReleaseSysCache(tup);

@@ -15468,12 +15469,12 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
             continue;

         /*
-         * When rebuilding an FK constraint that references the table we're
-         * modifying, we might not yet have any lock on the FK's table, so get
-         * one now.  We'll need AccessExclusiveLock for the DROP CONSTRAINT
-         * step, so there's no value in asking for anything weaker.
+         * When rebuilding another table's constraint that references the
+         * table we're modifying, we might not yet have any lock on the other
+         * table, so get one now.  We'll need AccessExclusiveLock for the DROP
+         * CONSTRAINT step, so there's no value in asking for anything weaker.
          */
-        if (relid != tab->relid && contype == CONSTRAINT_FOREIGN)
+        if (relid != tab->relid)
             LockRelationOid(relid, AccessExclusiveLock);

         ATPostAlterTypeParse(oldId, relid, confrelid,
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 476266e3f4b..750efc042d8 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4745,6 +4745,13 @@ alter table attbl alter column p1 set data type bigint;
 alter table atref alter column c1 set data type bigint;
 drop table attbl, atref;
 /* End test case for bug #17409 */
+/* Test case for bug #18970 */
+create table attbl(a int);
+create table atref(b attbl check ((b).a is not null));
+alter table attbl alter column a type numeric;  -- someday this should work
+ERROR:  cannot alter table "attbl" because column "atref.b" uses its row type
+drop table attbl, atref;
+/* End test case for bug #18970 */
 -- Test that ALTER TABLE rewrite preserves a clustered index
 -- for normal indexes and indexes on constraints.
 create table alttype_cluster (a int);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 5ce9d1e429f..41cff198e18 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -3069,6 +3069,15 @@ drop table attbl, atref;

 /* End test case for bug #17409 */

+/* Test case for bug #18970 */
+
+create table attbl(a int);
+create table atref(b attbl check ((b).a is not null));
+alter table attbl alter column a type numeric;  -- someday this should work
+drop table attbl, atref;
+
+/* End test case for bug #18970 */
+
 -- Test that ALTER TABLE rewrite preserves a clustered index
 -- for normal indexes and indexes on constraints.
 create table alttype_cluster (a int);

pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: Re: BUG #18970: Atempt to alter type of table column used in row type with check leads to assertion failure
Next
From: jian he
Date:
Subject: Re: BUG #18970: Atempt to alter type of table column used in row type with check leads to assertion failure