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: 
 <CACJufxH2NDhi4nTx_csqBao9i4bfxCyCXi7B8G=_XnWu8ru2TQ@mail.gmail.com>
References: <18970-a7d1cfe1f8d5d8d9@postgresql.org>
 <CACJufxGYBfGbOj3x0C0U-FsMR4K-gDEyxE6hjBJ870FnkDAO4w@mail.gmail.com>
 <2482012.1751216513@sss.pgh.pa.us> <2537374.1751218360@sss.pgh.pa.us>
 <CACJufxGKJtGNRRSXfwMW9SqVOPEMdP17BJ7DsBf=tNsv9pWU9g@mail.gmail.com>
 <2682232.1751250210@sss.pgh.pa.us>
 <CACJufxH2NDhi4nTx_csqBao9i4bfxCyCXi7B8G=_XnWu8ru2TQ@mail.gmail.com>
Comments: In-reply-to jian he <jian.universality@gmail.com>
	message dated "Mon, 30 Jun 2025 14:04:25 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2828228.1751296483.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Mon, 30 Jun 2025 11:14:43 -0400
Message-ID: <2828229.1751296483@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2828229.1751296483%40sss.pgh.pa.us>
Precedence: bulk

jian he <jian.universality@gmail.com> writes:
> SET EXPRESSION both can have this issue.
> so i also add more test cases for stored/virtual generated columns.

This patch is a lot larger than I was expecting, and I think it's
misguided.  You argue that

+     * Changing a virtual generated column's expression is akin to alteri=
ng its
+     * type, requiring a call to find_composite_type_dependencies to chec=
k if
+     * the virtual generated column is used in any table.
+     * Therefore we need add this defval to tab->newvals for virtual gene=
rated
+     * column too, so Phase3 will call find_composite_type_dependencies.

but I think that is in fact wrong.  The implementation restriction
we have is that we lack code to run around and physically change
the stored values of columns that are not top-level table columns.
However, in the case of a virtual column we don't need to change
anything about the storage: once we've fixed the catalog metadata,
we're done.  So I'm not seeing the need to add all this stuff
other than the additional locking calls.

			regards, tom lane