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

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: exclusion(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #18970: Atempt to alter type of table column used in row type with check leads to assertion failure
Date: 2025-07-01 03:25:20
Message-ID: CACJufxEZ2DzyAKNBLqW+-2BqFAbT5Wnxkgkw6ogcbBBsndkD=Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Mon, Jun 30, 2025 at 11:14 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> jian he <jian(dot)universality(at)gmail(dot)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 altering its
> + * type, requiring a call to find_composite_type_dependencies to check if
> + * the virtual generated column is used in any table.
> + * Therefore we need add this defval to tab->newvals for virtual generated
> + * 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.
>

CREATE TABLE t1(a int generated always as ('1') stored);
CREATE TABLE t2(b t1 CHECK ((b).a IS NOT NULL));
INSERT INTO t2 default values returning *;
ERROR: new row for relation "t2" violates check constraint "t2_b_check"
DETAIL: Failing row contains (null).
INSERT INTO t2 values '(1)' returning *;

ALTER TABLE t1 ALTER COLUMN a SET EXPRESSION AS (NULL);
Currently we disallow this "SET EXPRESSION AS".
I am wondering why we disallow it?

at that time, I was mainly initrucugged by this comment about
composite type default
in ATRewriteTables:
* If we change column data types, the operation has to be propagated
* to tables that use this table's rowtype as a column type.
* tab->newvals will also be non-NULL in the case where we're adding a
* column with a default. We choose to forbid that case as well,
* since composite types might eventually support defaults.

I’ve added some tests to generated_stored.sql, but not to generated_virtual.sql.

Attachment Content-Type Size
v3-0001-fix-bug-18970.patch text/x-patch 6.7 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Michael Paquier 2025-07-01 06:49:09 Re: BUG #18943: Return value of a function 'xmlBufferCreate' is dereferenced at xpath.c:177 without checking for NUL
Previous Message Tom Lane 2025-06-30 15:14:43 Re: BUG #18970: Atempt to alter type of table column used in row type with check leads to assertion failure