Re: Adding a stored generated column without long-lived locks

From: Xiaoxuan Li <xiaoxuan(dot)li(dot)dev(at)gmail(dot)com>
To: Alberto Piai <alberto(dot)piai(at)gmail(dot)com>
Cc: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Adding a stored generated column without long-lived locks
Date: 2026-11-13 20:06:28
Message-ID: CANuORAa25++iUj2qaQmxK3gkikxgX0nn=0v3h91Vqq3h8d1_LQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Alberto,

v12 (on e405a46c45b, Linux/aarch64, gcc 11, cassert) builds cleanly
and passes the regression tests, but this new change loses data:

> - clear atthasmissing and attmissingval for consistency with CREATE
> TABLE: a column that was GENERATED since the beginning can't have
> those set, so let's be consistent.

A column added with a constant default has no stored value in the rows
that existed before; they read attmissingval. The constraint is
validated against those values, and then the command removes them:

CREATE TABLE fd (a int);
INSERT INTO fd SELECT g FROM generate_series(1, 5) g;
ALTER TABLE fd ADD COLUMN c int DEFAULT 5;
INSERT INTO fd (a) VALUES (6);
ALTER TABLE fd ADD CONSTRAINT fd_ck
CHECK (pg_datum_image_equal(c, 5));
ALTER TABLE fd ALTER COLUMN c
ADD GENERATED USING CONSTRAINT fd_ck STORED;
SELECT a, c FROM fd ORDER BY a;
a | c
---+---
1 |
2 |
3 |
4 |
5 |
6 | 5

(Same with DEFAULT 0 and an expression such as a * 0.) Afterwards
fd_ck is still convalidated, VACUUM FULL keeps the NULLs, and
"UPDATE fd SET a = a WHERE a = 1" fails with a violation of fd_ck.

ATExecAlterColumnType() and ATExecSetExpression() also clear the
missing value, but only together with a table rewrite. There is none
here. v11 left both fields alone and returns 5 for every row above;
removing the two repl_repl[] assignments from v12 does the same, and
VACUUM FULL or SET EXPRESSION later resets atthasmissing as usual.

A test with ALTER TABLE ... ADD COLUMN ... DEFAULT would be good.

I have only looked at this part of the patch. I'll set the entry to
Waiting on Author.

--
Xiaoxuan Li
Amazon Web Services

On Fri, Nov 6, 2026 at 4:36 AM Alberto Piai <alberto(dot)piai(at)gmail(dot)com> wrote:

> On Fri Oct 2, 2026 at 1:30 AM CEST, Alberto Piai wrote:
> > On Thu Sep 24, 2026 at 12:47 PM CEST, Laurenz Albe wrote:
> >> On Thu, 2026-09-24 at 00:19 +0200, Alberto Piai wrote:
> >>> I find Matthias' proposal of exposing a function to check image
> equality
> >>> very compelling for the purpose of this patch: besides fixing this
> >>> problem, it would also make the command usable for data types which
> >>> don't define = (json), as well as those which don't (can't?) define
> >>> equalimage()... jsonb, numeric but also tsvector and PostGIS geometry.
> >>
> >> True, "tsvector" is limiting; I can see people wanting that for
> >> generated columns.
>
> I propose to move forward with pg_datum_image_equal() as the contract
> for the check constraint. It is simpler for the user. We can always
> reconsider the ROW()::record indirection in case the datum image
> equality patch cannot be committed for some reason.
>
> So here's v12, rebased on top of master + Matthias'
> pg_datum_image_equal() patch from [0]. It changes the implementation to
> require the new constraint shape, and updates all the documentation,
> tests and error messages.
>
> Besides that, I made a few more changes since v11:
>
> - bail if the column is part of a FK with on delete/update actions:
> adding the column directly in CREATE TABLE and ALTER TABLE ADD COLUMN
> also rejects that.
>
> - clear atthasmissing and attmissingval for consistency with CREATE
> TABLE: a column that was GENERATED since the beginning can't have
> those set, so let's be consistent.
>
> - remove support for foreign tables: I don't see the point of supporting
> this, since adding a stored generated column on a foreign table does
> not involve a rewrite nor a scan. The actual data migration would have
> to be done on the remote anyway.
>
> - fix wrong type for a pg_depend.refobjsubid lookup.
>
> - release AccessShareLock after scanning pg_depend: it's inconsequential
> since the target rel is already under AccessExclusiveLock, so no
> concurrent DDL involving it would happen. But it makes the function
> more self contained.
>
> - fix detection of trivial constraint col1=col2 (two Vars, but in
> swapped order).
>
> - add a tsvector test, also for dump/restore.
>
> - a few minor fixes, such as an incorrect use of errmsg instead of
> errmsg_internal and an incorrect comment.
>
>
> Looking forward to your thoughts on this!
>
>
> Regards,
>
> Alberto
>
> [0]
> https://postgr.es/m/CAEze2WhW41mVDMKRcpLaj_NMLVp4jtQLZjtG3M8RXNNbhZmCcw@mail.gmail.com
>
>
> --
> Alberto Piai
> Sensational AG
> Zürich, Switzerland
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Fomin 2026-11-13 20:24:34 Re: [PATCH v1 0/7] Wait event timing and tracing instrumentation
Previous Message Greg Burd 2026-11-13 19:45:53 Re: Keep TIDBitmap pagetable memory across bitmap scans