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 |
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 |