From: Tom Lane <tgl@sss.pgh.pa.us>
To: jian he <jian.universality@gmail.com>
cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18970: Atempt to alter type of table column used in row type
 with check leads to assertion failure
In-reply-to: <2482012.1751216513@sss.pgh.pa.us>
References: <18970-a7d1cfe1f8d5d8d9@postgresql.org>
 <CACJufxGYBfGbOj3x0C0U-FsMR4K-gDEyxE6hjBJ870FnkDAO4w@mail.gmail.com>
 <2482012.1751216513@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Sun, 29 Jun 2025 13:01:53 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <2537367.1751218356.0@sss.pgh.pa.us>
Date: Sun, 29 Jun 2025 13:32:40 -0400
Message-ID: <2537374.1751218360@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2537374.1751218360%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2537367.1751218356.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-bug-18970.patch"; charset="us-ascii"
Content-ID: <2537367.1751218356.2@sss.pgh.pa.us>
Content-Description: fix-bug-18970.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/table=
cmds.c
index 991bc946ffc..b8837f26cb4 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15415,9 +15415,12 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTabl=
eInfo *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 i=
n
-	 * 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 nee=
d
+	 * 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 oth=
er
+	 * 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, AlteredTable=
Info *tab, LOCKMODE lockmode)
 		Form_pg_constraint con;
 		Oid			relid;
 		Oid			confrelid;
-		char		contype;
 		bool		conislocal;
 =

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

@@ -15468,12 +15469,12 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTab=
leInfo *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 !=3D tab->relid && contype =3D=3D CONSTRAINT_FOREIGN)
+		if (relid !=3D 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 big=
int;
 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 wo=
rk
+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/a=
lter_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 wo=
rk
+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);

------- =_aaaaaaaaaa0--