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: 
 <CACJufxGYBfGbOj3x0C0U-FsMR4K-gDEyxE6hjBJ870FnkDAO4w@mail.gmail.com>
References: <18970-a7d1cfe1f8d5d8d9@postgresql.org>
 <CACJufxGYBfGbOj3x0C0U-FsMR4K-gDEyxE6hjBJ870FnkDAO4w@mail.gmail.com>
Comments: In-reply-to jian he <jian.universality@gmail.com>
	message dated "Sun, 29 Jun 2025 13:28:05 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <2482011.1751216513.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 29 Jun 2025 13:01:53 -0400
Message-ID: <2482012.1751216513@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2482012.1751216513%40sss.pgh.pa.us>
Precedence: bulk

jian he <jian.universality@gmail.com> writes:
> On Sun, Jun 29, 2025 at 1:35=E2=80=AFAM PG Bug reporting form
> <noreply@postgresql.org> wrote:
>> CREATE TABLE t1(a int);
>> CREATE TABLE t2(b t1 CHECK ((b).a IS NOT NULL));
>> ALTER TABLE t1 ALTER COLUMN a TYPE numeric;
>> triggers
>> 2025-06-28 06:52:21.201 UTC [2233016] LOG:  statement: ALTER TABLE t1 A=
LTER
>> COLUMN a TYPE numeric;
>> TRAP: failed Assert("lockmode !=3D NoLock || IsBootstrapProcessingMode(=
) ||
>> CheckRelationLockedByMe(r, AccessShareLock, true)"), File: "relation.c"=
,
>> Line: 67, PID: 2233016

> in ATPostAlterTypeCleanup
>         /*
>          * 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 CONSTRA=
INT
>          * step, so there's no value in asking for anything weaker.
>          */
>         if (relid !=3D tab->relid && contype =3D=3D CONSTRAINT_FOREIGN)
>             LockRelationOid(relid, AccessExclusiveLock);

> we can change to
>         if (relid !=3D tab->relid)
>             LockRelationOid(relid, AccessExclusiveLock);
> obviously, the comments need to be updated.

Yeah, I came to the same conclusion after studying it for awhile.
The problem is that because of the "(b).a" column reference in
the CHECK expression, we record a direct dependency of t2's CHECK
constraint on t1.a, and ATPostAlterTypeCleanup is evidently not
expecting that.

> When altering the data type of a column in one relation causes a constra=
int of
> another table rebuild, the other table should be locked with
> AccessExclusiveLock.

I wonder if there might be cases where a lesser lock is sufficient.
However, this is apparently a very edgy edge case, so it's probably
not worth obsessing over the lock level too much.

It's somewhat annoying that we're just going to fail later.
I thought about whether find_composite_type_dependencies
ought to be run sooner, so that we'd error out before potentially
doing a lot of work.  But I think the design idea is that
someday find_composite_type_dependencies would actually propagate
the changes, in which case its current placement is correct.

			regards, tom lane