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 <[email protected]> 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_nmlvp4jtqlzjtg3m8rxnnbhzm...@mail.gmail.com
>
>
> --
> Alberto Piai
> Sensational AG
> Zürich, Switzerland
>

Reply via email to