From: Tom Lane <tgl@sss.pgh.pa.us>
To: jian he <jian.universality@gmail.com>
Cc: exclusion@gmail.com
Cc: 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
Date: Sun, 29 Jun 2025 13:32:40 -0400
Message-ID: <2537374.1751218360@sss.pgh.pa.us> (raw)
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>
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
Attachments:
[text/x-diff] fix-bug-18970.patch (4.2K, ../2537374.1751218360@sss.pgh.pa.us/2-fix-bug-18970.patch)
download | inline diff:diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 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.outindex 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.sqlindex 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);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, jian.universality@gmail.com, 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: <2537374.1751218360@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox