Adding a stored generated column without long-lived locks

Hi,

I recently needed to add a stored generated column to a table of
nontrivial size, and realized that currently there is no way to do that
without rewriting the table under an AccessExclusiveLock.

One way I think this could be achieved:

- allow turning an existing column into a stored generated column, by
default doing a table rewrite using the new stored column expression

- when doing the above, try to detect the presence of a check constraint
which proves that the contents of the column already match its defined
expression, and in that case skip the rewrite

This would open up a path to add such a column (GENERATED ALWAYS AS
(expr) STORED) without long-lived locks:

- add column c, nullable
- add trigger to set c = expr for new/updated rows
- add constraint check (c = expr) NOT VALID
- backfill the table at the appropriate pace
- VALIDATE the constraint
- alter the column c to be GENERATED ALWAYS AS (expr) STORED, which
would skip the rewrite because of the valid check constraint on c
- clean up the trigger and the constraint

To this effect, I started prototyping an alter table command

ALTER TABLE t ALTER COLUMN c ADD GENERATED ALWAYS AS (expr) STORED

The syntax seemed like a good fit because it's similar to the command to
change a column to be GENERATED AS IDENTITY, but I didn't spend a whole
lot of thought on the exact syntax yet.

The attached patches are a first prototype for discussion:

- patch v1-0001: add the command
- patch v1-0002: detect the check constraint and skip the rewrite

The check constraint must be of the form

(c = <expr>)

where `=` is a mergejoinable operator for the type c.

The <expr> in the constraint and in the column definition are matched
structurally, so they must match exactly.

Before spending more time on this, I wanted to bring this up for
discussion and to gauge interest in the idea.

Looking forward to your feedback!

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Tue Mar 17, 2026 at 5:31 PM +07, Alberto Piai wrote:

> I recently needed to add a stored generated column to a table of
> nontrivial size, and realized that currently there is no way to do
> that without rewriting the table under an AccessExclusiveLock.

[...]

> To this effect, I started prototyping an alter table command

We currently have a way to change the expression of generated columns
(SET EXPRESSION) and a way to turn a generated column into a regular one
(DROP EXPRESSION). The new command would fit nicely and provide the
missing piece of functionality: turning an existing column into a
generated column.

A few thoughts:

- since this is specifically useful for *stored* generated columns (to
have a way to avoid a rewrite while the table is locked), I would
stick to my first proposal and require that STORED is specified
explicitly. It would still be possible to remove this requirement and
expand to virtual generated columns, should the need for this arise in
the future (I just don't see the use case right now).

- realizing that this is the opposite operation of DROP EXPRESSION gave
me a clue about how to support partitioning/inheritance.
AT_DropExpression can be applied only to the whole inheritance tree at
once (see 8bf6ec3ba3a44448817af47a080587f3b71bee08 and the associated
discussion at https://postgr.es/m/2793383.1672944799@sss.pgh.pa.us)
it refuses to be applied to either the parent table ONLY, or directly
to partitions. This new command should work the same way.

- while researching the above, I stumbled upon a restriction of current
DROP EXPRESSION: it doesn't seem to be possible to apply it to
partition trees deeper than just one level (parent / child tables).
This is probably an oversight, but to avoid feature-creeping this
patch, I made the new command act the same way (see test case). I'll
try to address this separately.

- I added some note in the commit message to clarify why I added the new
command to AT_PASS_SET_EXPRESSION, since this wasn't clear enough in
my first mail/patch.

- I am not particularly attached to the syntax. Alternatives that would
come to mind would be:

SET GENERATED ALWAYS AS (expr) STORED

or to match the two existing commands:

ADD EXPRESSION (expr) STORED

As I said above, I think the explicit STORED is necessary. It would be
nice if the command would make it crystal clear to the user that it
implies rewriting the table, i.e. overwriting existing data. (To me,
all three forms are clear enough, especially considering that by this
point I would have already typed ALTER twice :-))

The attached v2 patches take care of the points above. They are again
split in two commits for ease of review.

Looking forward to any comment / feedback!

Alberto

PS: A note about the timing of this mail, as I am just getting
acquainted with all of this. I am aware that we're super short of a
feature freeze, and this thread is by no means an attempt to push for
this to go in now, nor to steal brain bandwidth from more important
active threads. I just thought it's OK to put the patches and the mails
out there as I make progress, even if it's just to bring this up and
revisit at a later point in time. Let me know if instead it would be
better to sit on my thoughts until a more appropriate time in the
release cycle.

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Tue Apr 7, 2026 at 5:02 PM +08, Alberto Piai wrote:
> On Tue Mar 17, 2026 at 5:31 PM +07, Alberto Piai wrote:
>
>> I recently needed to add a stored generated column to a table of
>> nontrivial size, and realized that currently there is no way to do
>> that without rewriting the table under an AccessExclusiveLock.

Attached v3, just a rebase onto current master.

Regards,

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Fri Apr 24, 2026 at 2:10 AM PDT, Alberto Piai wrote:
> On Tue Apr 7, 2026 at 5:02 PM +08, Alberto Piai wrote:
>> On Tue Mar 17, 2026 at 5:31 PM +07, Alberto Piai wrote:
>>
>>> I recently needed to add a stored generated column to a table of
>>> nontrivial size, and realized that currently there is no way to do
>>> that without rewriting the table under an AccessExclusiveLock.

The attached v4 is a rebase against current master, plus:

- I moved the call to RememberAllDependentForRebuilding before the
update to pg_attribute, since it provides checks for some
invalid/unsupported invocations it seems more useful to do it before
changing anything.

- I hadn't noticed before that AddRelationNewConstraints returns the new
(cooked) default definitions, so we can use those instead of building
them again.

- cleaned up some includes I had added by mistake, and moved some tests
around between the two commits

A while back I also posted a fix for the issue of DROP EXPRESSION not
working with subpartitions [1], this patch isn't ajusted yet to match, I
would do that if the bugfix would be committed first.

I am still hoping to get a reviewer for the in-person commitfest at the
upcoming pgconf.dev :)

It's my first contribution, but the change is pretty self-contained and
hopefully not terribly complex to review. I'm trying to address a real
world use case, it would be fantastic to make some progress with this
patch.

Anyone's motivated? :)

Regards,

Alberto

[1] https://www.postgresql.org/message-id/DHMT78XOD8BK.341V3H87KZ7NO%40gmail.com

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Thu, 2026-05-14 at 15:46 -0700, Alberto Piai wrote:
> > > On Tue Mar 17, 2026 at 5:31 PM +07, Alberto Piai wrote:
> > >
> > > > I recently needed to add a stored generated column to a table of
> > > > nontrivial size, and realized that currently there is no way to do
> > > > that without rewriting the table under an AccessExclusiveLock.
>
> The attached v4 is a rebase against current master

I understand the need that the patch fulfills, and I agree that it would be a
nice feature.

I have a few thoughts about this that don't concern the implementation:

1) The SQL standard knows ALTER TABLE ... ADD ... GENERATED ALWAYS AS (...)
and ALTER TABLE ... ALTER ... DROP EXPRESSION, but there is no provision
for ALTER TABLE ... ADD GENERATED ALWAYS AS (...).
So this patch adds non-standard syntax that may one day conflict with
a new version of the standard. I think we can still do it, and the
proposed syntax looks right, but I thought I should mention it.

2) We currently have ALTER TABLE ... ALTER ... SET EXPRESSION AS (...) to
change the generation expression of a column. This command always
rewrites the table, according to the documentation.
I think that if the present patch adds support to skip rewriting the table
when a generation expression is added and the expression matches a check
constraint, changing the generation expression should also be possible
without a rewrite. If not, I would consider that a violation of the
principle of least astonishment.
Would it be difficult to extend the patch to support that?

3) We already have a couple of tricks to avoid blocking for a long time:

- ALTER TABLE ... ALTER ... SET NOT NULL can skip the table scan if there
is a check constraint that makes sure that the column is NOT NULL

- ALTER TABLE ... ATTACH PARTITION can skip the scan of the new partition
if there is a check constraint matching the partition constraint

It would be great to document these little tricks in the documentation,
probably on the ALTER TABLE page. This is not necessarily the job of
this patch, but it would also not be off-topic for the patch.

Comments on the patch:
----------------------

The patch applies and builds cleanly and passes the regression tests.

Missing parts:

- There is no documentation. At least ALTER TABLE needs a description of the
new syntax, and would ideally mention the trick with the check constraint.

- There should be support for command line completion for the new syntax.

Bugs:

- The patch doesn't test if the column is an identity column:

CREATE TABLE tab (id bigint GENERATED ALWAYS AS IDENTITY PRIMARY KEY);
INSERT INTO tab VALUES (DEFAULT);
ALTER TABLE tab ALTER id ADD GENERATED ALWAYS AS (1) STORED;

The ALTER TABLE should fail, but doesn't.

- Strange behavior with sequences owned by the column:

CREATE TABLE tab (id bigserial);
INSERT INTO tab VALUES (DEFAULT);
ALTER TABLE tab ALTER id ADD GENERATED ALWAYS AS (1) STORED;
\ds tab_id_seq
List of sequences
Schema | Name | Type | Owner
--------+------------+----------+----------
public | tab_id_seq | sequence | postgres
(1 row)

I think that any sequence owned by the column should be dropped.
Alternatively, you could throw an error.

- Incorrect handling of NULL values:

CREATE TABLE tab (col1 integer, col2 integer);
INSERT INTO tab VALUES (2, NULL);
-- works, because NULL results from the check are accepted
ALTER TABLE tab ADD CHECK (col2 = col1);

SELECT pg_relation_filenode('tab');
pg_relation_filenode
----------------------
19920
(1 row)

ALTER TABLE tab ALTER col2 ADD GENERATED ALWAYS AS (col1) STORED;
SELECT pg_relation_filenode('tab');
pg_relation_filenode
----------------------
19920
(1 row)

TABLE tab;
col1 | col2
------+------
2 | ∅
(1 row)

I am not sure what the correct approach would be. The simple approach would be
to only skip the rewrite if the column has a NOT NULL constraint or an equivalent
check constraint, but perhaps you can think of a way to do better.

Comments on the code:

> --- a/src/backend/commands/tablecmds.c
> +++ b/src/backend/commands/tablecmds.c
> @@ -5093,6 +5102,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
> ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
> pass = AT_PASS_SET_EXPRESSION;
> break;
> + case AT_AddGeneratedAsExprStored:

You should add a comment, same as for the other branches.

> @@ -6695,6 +6717,8 @@ alter_table_type_to_string(AlterTableType cmdtype)
> return "ALTER COLUMN ... SET NOT NULL";
> case AT_SetExpression:
> return "ALTER COLUMN ... SET EXPRESSION";
> + case AT_AddGeneratedAsExprStored:
> + return "ALTER COLUMN ... ADD GENERATED ALWAYS AS (...) STORED";

Keep it short, like "ALTER COLUMN ... ADD GENERATED".

> --- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
> +++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
> @@ -129,6 +129,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
> case AT_SetNotNull:
> strtype = "SET NOT NULL";
> break;
> + case AT_AddGeneratedAsExprStored:
> + strtype = "ADD GENERATED ALWAYS AS (...) STORED";
> + break;

I suggest "ALTER COLUMN ADD GENERATED ALWAYS AS", but I won't insist.

Yours,
Laurenz Albe

Hi,

On Fri May 15, 2026 at 12:46 AM CEST, Alberto Piai wrote:
> On Fri Apr 24, 2026 at 2:10 AM PDT, Alberto Piai wrote:
>> On Tue Apr 7, 2026 at 5:02 PM +08, Alberto Piai wrote:
>>> On Tue Mar 17, 2026 at 5:31 PM +07, Alberto Piai wrote:
>>>
>>>> I recently needed to add a stored generated column to a table of
>>>> nontrivial size, and realized that currently there is no way to do
>>>> that without rewriting the table under an AccessExclusiveLock.

here's a not-so-brief summary of the conversations around this topic at
pgconf.dev, and a new proposal at the end.

I had the chance to bring this up with other attendees, and many
recognized the use case as a useful one, addressing a real operational
issue.

In particular, I had great feedback from Staš Kotarac Guček, who pointed
out a major flaw in my current proposal: a constraint of the form

CHECK (c = expr)

would not work correctly when expr evaluates to null for some rows.
Thank you Staš, in the next iteration I will change the constraint to
use IS NOT DISTINCT FROM, instead.

I briefly mentioned this topic to Tom Lane, who quickly replied with the
question: should this not fail when it can't use the constraint, instead
of overwriting the contents of the column?

Thanks Tom, I will get to this later in this mail.

I had registered this patch for the in-person commitfest at pgconf.dev,
and Álvaro Herrera picked it up for review. Thank you Álvaro, and thank
you Peter for organizing the event.

We managed to find some time on the very last day of the conference, and
went through the current design and code. The open items (which I will
address in the next iteration of this patch) are:

* missing user documentation

I will work on this next. I think it's a good way to explain the
feature even early during development. I just didn't want to do it
_too_ early, without having had any feedback.

* try to minimize command counter increments

There might be one call to CommandCounterIncrement() which is not
necessary, I'll try remove it.

* comment on why it is necessary to clear missing values when rewriting
the table

ATExecAlterColumnType() and ATExecSetExpression() both do this
explicitly when requesting a table rewrite. I'll extend the comment,
and also look into whether this is something that should be done any
time a table rewrite happens. In that case, it might be worth moving
this into the rewriting code rather than having each caller do it.

* interactions with other subcommands in the same alter table statement

My reasoning regarding this was: if I do this in
AT_PASS_SET_EXPRESSION, it should be safe. I will invest some more
time into this and add tests, too.

We also looked at the overall design of the new command, and we agreed
that it is a fitting addition to our current SET EXPRESSION and DROP
EXPRESSION. Regarding the question of whether it should be SET or ADD,
we agreed that ADD (i.e. the current proposal) is clearer, especially
for its similarity to ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY.

Regarding the question of "should this fail or rewrite the table when a
usable constraint isn't found": Álvaro's suggestion here was to use a
more ad-hoc command, meant more specifically for this use case of
converting into a stored generated column without rewriting it. If the
command would be dedicated specifically to this, it would make sense to
have it fail when a usable constraint isn't found.

Last but not least, I also discussed this with Laurenz Albe, and he
wrote a very useful review in this thread. I will address that
separately and reply directly to that mail, but one point I can already
merge in this discussion is about the syntax of the command:

> 1) The SQL standard knows ALTER TABLE ... ADD ... GENERATED ALWAYS AS (...)
> and ALTER TABLE ... ALTER ... DROP EXPRESSION, but there is no provision
> for ALTER TABLE ... ADD GENERATED ALWAYS AS (...).
> So this patch adds non-standard syntax that may one day conflict with
> a new version of the standard. I think we can still do it, and the
> proposed syntax looks right, but I thought I should mention it.

I'd like to take his point, together with the question from Tom and the
suggestion by Álvaro, and make a new proposal for the design of this
command.

Design iteration 2
------------------

Syntax:

ALTER TABLE t ALTER COLUMN c
ADD GENERATED ALWAYS AS (expr) STORED USING CONSTRAINT check_name

check_name must be a valid constraint of the form

CHECK (c IS NOT DISTINCT FROM (expr))

This fails if:

- a check constraint named check_name is not found for table c
- the constraint is not valid
- the constraint does not match exactly the expr the user intends to use
as a stored default expression

On success, the table c is now a stored generated column with the given
default expression, and the check_name constraint has been removed.

This addresses Tom's remark, we can now fail instead of just rewriting
the column.

It improves slightly upon the issue of a potential conflict with a
future edition of the SQL standard, by being more specific. I don't see
a way to be completely sure we won't have conflicts. We could improve
more by making the syntax more "alien" and very unlinkely to be picked
up by the standard, but at a usability cost for Postgres. I'm open to
suggestions.

It improves upon another question raised by Álvaro: does the user have
to clean up the constraint? In v1 I felt it was better to have the user
remove it after the migration. Since here it's explicitly mentioned as
the constraint to use to migrate the column, I think it's OK to remove
it. We are conceptually moving it from being a constraint to being the
new default expression.

The implementation should also be simpler, since there will never be a
table rewrite.

Any thoughts about this?

Best regards,

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Tue May 26, 2026 at 5:23 PM CEST, Laurenz Albe wrote:
>
> 1) The SQL standard knows ALTER TABLE ... ADD ... GENERATED ALWAYS AS (...)
> and ALTER TABLE ... ALTER ... DROP EXPRESSION, but there is no provision
> for ALTER TABLE ... ADD GENERATED ALWAYS AS (...).
> So this patch adds non-standard syntax that may one day conflict with
> a new version of the standard. I think we can still do it, and the
> proposed syntax looks right, but I thought I should mention it.

Thank you for bringing this up. I don't have access to the standard, but
the chance of a possible conflict with future editions was at the back
of my mind. I don't see a way to exclude it completely. In a sibling
mail in this thread (you should be in CC), I have made a new iteration
on this proposal, which also tries to make the command more specific to
avoid future conflicts.

> 2) We currently have ALTER TABLE ... ALTER ... SET EXPRESSION AS (...) to
> change the generation expression of a column. This command always
> rewrites the table, according to the documentation.
> I think that if the present patch adds support to skip rewriting the table
> when a generation expression is added and the expression matches a check
> constraint, changing the generation expression should also be possible
> without a rewrite. If not, I would consider that a violation of the
> principle of least astonishment.
> Would it be difficult to extend the patch to support that?

Yes, I don't see a way to make that work. Since we're talking only about
stored values, a rewrite will always be necessary. However, using this
new command, a user could add a column with the new expression, then
atomically drop the old one and rename. All without holding onto an
AccessExclusiveLock for a long time :)

> 3) We already have a couple of tricks to avoid blocking for a long time:
>
> - ALTER TABLE ... ALTER ... SET NOT NULL can skip the table scan if there
> is a check constraint that makes sure that the column is NOT NULL
>
> - ALTER TABLE ... ATTACH PARTITION can skip the scan of the new partition
> if there is a check constraint matching the partition constraint
>
> It would be great to document these little tricks in the documentation,
> probably on the ALTER TABLE page. This is not necessarily the job of
> this patch, but it would also not be off-topic for the patch.

The SET NOT NULL one and the ATTACH PARTITION one are documented in the
section specific to the command. However

or, if an equivalent index already exists, it will be attached to the
target table's index, as if ALTER INDEX ATTACH PARTITION had been
executed

is not very explicit about the advantages this has for online
migrations.

In the NOTES section of the ALTER TABLE page, there is a paragraph about
NOT VALID / VALIDATE, which is another operation in the same spirit as
this.

Maybe we could group them all in a new section dedicated to online
schema migrations?

(However, even if it's definitely on-topic with this patch, I would work
on this in a separate patch / email thread.)

> Missing parts:
>
> - There is no documentation. At least ALTER TABLE needs a description of the
> new syntax, and would ideally mention the trick with the check constraint.

Yes, I will work on this next. I also believe it's a great way to show a
feature, even early during development. I just wanted to avoid doing it
_too_ early, before having had any feedback about the idea.

> - There should be support for command line completion for the new syntax.

Great idea, I'll add this too.

> Bugs:
>
> - The patch doesn't test if the column is an identity column:
>
> CREATE TABLE tab (id bigint GENERATED ALWAYS AS IDENTITY PRIMARY KEY);
> INSERT INTO tab VALUES (DEFAULT);
> ALTER TABLE tab ALTER id ADD GENERATED ALWAYS AS (1) STORED;
>
> The ALTER TABLE should fail, but doesn't.
>
> - Strange behavior with sequences owned by the column:
>
> CREATE TABLE tab (id bigserial);
> INSERT INTO tab VALUES (DEFAULT);
> ALTER TABLE tab ALTER id ADD GENERATED ALWAYS AS (1) STORED;
> \ds tab_id_seq
> List of sequences
> Schema | Name | Type | Owner
> --------+------------+----------+----------
> public | tab_id_seq | sequence | postgres
> (1 row)
>
> I think that any sequence owned by the column should be dropped.
> Alternatively, you could throw an error.

Thanks for testing this!

I have reused RememberAllDependentForRebuilding() which does some
validation, but was originally meant for ALTER COLUMN TYPE. I will add
checks and tests for these cases, but to be consistent with how the
other dependencies are handled, I think it's better to throw an error
here (this is what happens for example if trying to ALTER TYPE of a
column used by a function).

>
> - Incorrect handling of NULL values:

See sibling mail, in the next iteration the constraint will have to use
IS NOT DISTINCT FROM. I think that should cover all cases.

>
> Comments on the code:
>
>> --- a/src/backend/commands/tablecmds.c
>> +++ b/src/backend/commands/tablecmds.c
>> @@ -5093,6 +5102,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
>> ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
>> pass = AT_PASS_SET_EXPRESSION;
>> break;
>> + case AT_AddGeneratedAsExprStored:
>
> You should add a comment, same as for the other branches.
>
>> @@ -6695,6 +6717,8 @@ alter_table_type_to_string(AlterTableType cmdtype)
>> return "ALTER COLUMN ... SET NOT NULL";
>> case AT_SetExpression:
>> return "ALTER COLUMN ... SET EXPRESSION";
>> + case AT_AddGeneratedAsExprStored:
>> + return "ALTER COLUMN ... ADD GENERATED ALWAYS AS (...) STORED";
>
> Keep it short, like "ALTER COLUMN ... ADD GENERATED".
>
>> --- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
>> +++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
>> @@ -129,6 +129,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
>> case AT_SetNotNull:
>> strtype = "SET NOT NULL";
>> break;
>> + case AT_AddGeneratedAsExprStored:
>> + strtype = "ADD GENERATED ALWAYS AS (...) STORED";
>> + break;
>
> I suggest "ALTER COLUMN ADD GENERATED ALWAYS AS", but I won't insist.

Agreed, will fix all these in the next version of the patch.

Thank you again for the review!

Best regards,

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Wed, 2026-05-27 at 19:43 +0200, Alberto Piai wrote:
> I'd like to take his point, together with the question from Tom and the
> suggestion by Álvaro, and make a new proposal for the design of this
> command.
>
> Design iteration 2
> ------------------
>
> Syntax:
>
> ALTER TABLE t ALTER COLUMN c
> ADD GENERATED ALWAYS AS (expr) STORED USING CONSTRAINT check_name
>
> check_name must be a valid constraint of the form
>
> CHECK (c IS NOT DISTINCT FROM (expr))
>
> This fails if:
>
> - a check constraint named check_name is not found for table c
> - the constraint is not valid
> - the constraint does not match exactly the expr the user intends to use
> as a stored default expression
>
> On success, the table c is now a stored generated column with the given
> default expression, and the check_name constraint has been removed.
>
> This addresses Tom's remark, we can now fail instead of just rewriting
> the column.

I like this proposal. It avoids the question "to rewrite or not to
rewrite" by just outright failing if there is no suitable constraint.

The idea to avoid the problem with NULL by forcing IS NOT DISTINCT FROM
in the constraint is a good solution. Perhaps you could also allow
the equality operator if the column in question is defined NOT NULL.

> It improves slightly upon the issue of a potential conflict with a
> future edition of the SQL standard, by being more specific. I don't see
> a way to be completely sure we won't have conflicts. We could improve
> more by making the syntax more "alien" and very unlinkely to be picked
> up by the standard, but at a usability cost for Postgres. I'm open to
> suggestions.

I don't think that the new proposal makes it less likely to get in
conflict with later additions to the standard. But I don't think that
inventing unlikely syntax to avoid such conflicts is a good idea.
First, the standard committee itself seems (or seemed) to have a strong
predilection for alien, verbose syntax.
Second, if we end up with weird, unwieldy syntax, that would be bad.

No, the syntax you are proposing sounds reasonable to me.

> It improves upon another question raised by Álvaro: does the user have
> to clean up the constraint? In v1 I felt it was better to have the user
> remove it after the migration. Since here it's explicitly mentioned as
> the constraint to use to migrate the column, I think it's OK to remove
> it. We are conceptually moving it from being a constraint to being the
> new default expression.
>
> The implementation should also be simpler, since there will never be a
> table rewrite.
>
> Any thoughts about this?

Yes. I think that you should not drop the constraint. That's what I'd
expect, similar to how we don't drop the check constraint that allows
to skip the table scan in ALTER TABLE ... ALTER COLUMN ... SET NOT NULL
or ALTER TABLE ... ATTACK PARTITION.

I feel that automatically dropping the constraint is a bit too much
black magic, but it is more a feeling than a conviction.

Yours,
Laurenz Albe

On Wed, 2026-05-27 at 19:44 +0200, Alberto Piai wrote:
> On Tue May 26, 2026 at 5:23 PM CEST, Laurenz Albe wrote:
>
>
> > 2) We currently have ALTER TABLE ... ALTER ... SET EXPRESSION AS (...) to
> > change the generation expression of a column. This command always
> > rewrites the table, according to the documentation.
> > I think that if the present patch adds support to skip rewriting the table
> > when a generation expression is added and the expression matches a check
> > constraint, changing the generation expression should also be possible
> > without a rewrite. If not, I would consider that a violation of the
> > principle of least astonishment.
> > Would it be difficult to extend the patch to support that?
>
> Yes, I don't see a way to make that work. Since we're talking only about
> stored values, a rewrite will always be necessary. However, using this
> new command, a user could add a column with the new expression, then
> atomically drop the old one and rename. All without holding onto an
> AccessExclusiveLock for a long time :)

With your new proposal to never rewrite the table, but fail instead if
there is no constraint, my objection loses its point, so I withdraw it.

> > 3) We already have a couple of tricks to avoid blocking for a long time:
> >
> > - ALTER TABLE ... ALTER ... SET NOT NULL can skip the table scan if there
> > is a check constraint that makes sure that the column is NOT NULL
> >
> > - ALTER TABLE ... ATTACH PARTITION can skip the scan of the new partition
> > if there is a check constraint matching the partition constraint
> >
> > It would be great to document these little tricks in the documentation,
> > probably on the ALTER TABLE page. This is not necessarily the job of
> > this patch, but it would also not be off-topic for the patch.
>
> The SET NOT NULL one and the ATTACH PARTITION one are documented in the
> section specific to the command. However
>
> or, if an equivalent index already exists, it will be attached to the
> target table's index, as if ALTER INDEX ATTACH PARTITION had been
> executed
>
> is not very explicit about the advantages this has for online
> migrations.
>
> In the NOTES section of the ALTER TABLE page, there is a paragraph about
> NOT VALID / VALIDATE, which is another operation in the same spirit as
> this.
>
> Maybe we could group them all in a new section dedicated to online
> schema migrations?

You are right, the existing shortcuts are documented. Your new proposal
makes the proposed feature different from these existing cases, so I don't
think lumping them together is a good idea now.

> Agreed, will fix all these in the next version of the patch.

Great; I'm looking forward to it.

Yours,
Laurenz Albe

On Mon Jun 15, 2026 at 8:38 PM CEST, Laurenz Albe wrote:
> On Wed, 2026-05-27 at 19:43 +0200, Alberto Piai wrote:
>> Design iteration 2
>> ------------------
>
> I like this proposal. It avoids the question "to rewrite or not to
> rewrite" by just outright failing if there is no suitable constraint.
>
> The idea to avoid the problem with NULL by forcing IS NOT DISTINCT FROM
> in the constraint is a good solution. Perhaps you could also allow
> the equality operator if the column in question is defined NOT NULL.

In the attached v5 patch I've implemented this design, and went one step
further (let me know what you think). While discussing this with my
colleagues at work, the question came up (thanks, Philip!): now that we
mention the constraint explicitly, what's the point of repeating the
expression too? The constraint already defines an equality to an
expression. I think this is a very good point, and it removes one
further way in which the operation could fail, so I went ahead and
changed the command to not mention the expression. It takes the
expression defined in the constraint and uses _that_ as the generator
expression of the column.

Design iteration 3
------------------

Syntax:

ALTER TABLE t ALTER COLUMN c
ADD GENERATED ALWAYS STORED USING CONSTRAINT check_name

check_name must be a valid constraint of a specific shape. If the
column is nullable:

CHECK (c IS NOT DISTINCT FROM expr)

If the column is NOT NULL, either of the following is acceptable:

CHECK (c = expr)
CHECK (c IS NOT DISTINCT FROM expr)

The column is then changed to be a stored generated column, with the
"expr" from the constraint as its generator expression.

>> Any thoughts about this?
>
> Yes. I think that you should not drop the constraint. That's what I'd
> expect, similar to how we don't drop the check constraint that allows
> to skip the table scan in ALTER TABLE ... ALTER COLUMN ... SET NOT
> NULL or ALTER TABLE ... ATTACK PARTITION.
>
> I feel that automatically dropping the constraint is a bit too much
> black magic, but it is more a feeling than a conviction.

I don't have a strong opinion on whether to cleanup or not, I'll gladly
take your input. This version of the patch does not drop the constraint
anymore.

This version addresses your inputs from the last review:

- I added documentation for the new alter table form to alter_table.sgml
- Tab completion for psql is there now
- The missing error conditions in case of an identity column or
sequences are now handled, more about this in the next section.

Failure conditions
------------------

There's quite a few invalid states that cannot be reached via CREATE
TABLE and should not be reachable via ALTER TABLE either.

The following are detected and fail the operation:

- the column is already a generated column

- the column is an identity column

- the column is referenced by a sequence (it is most likely a serial
column)

- the column is referenced by another column's default expression

- the column references another generated column

- the column is referenced in a partition key, either directly or
through a whole row expression

- the new default expression is not immutable

Additionally, we of course bail if the constraint is not found, not
valid, not enforced or doesn't match the specific structure we need.

Another case I considered is the column being referenced in the body of
a pre-parsed function (BEGIN ATOMIC SQL functions). In this case though,
it seems to me that we don't need to fail here: we are not altering the
type of the column, and when reading a stored generated column there's
no expression replacement happening (as it does when reading virtual
columns).

Partitioning/inheritance is supported only on the whole hierarchy at
once (see 8bf6ec3ba3a44448817af47a080587f3b71bee08). Trying to change
the column at only one level will fail, as well as any of the subtrees.

I also added a test to explictily check that we're not accidentally
enqueuing a table scan for verification in phase 3, as avoiding this
kind of work is the whole point of the command.

Logical replication
-------------------

The interaction with logical replication is tricky, since a publication
can have the option to publish generated columns or not (which is the
default).

When not publishing stored generated columns, inserts or updates would
be replicated while backfilling, and would then suddenly stop when the
column is turned into a stored generated column.

One way to avoid this is to set up triggers on the subscriber too,
before altering the column on the publisher. This way updates and
inserts would not lose the column's value on the subscriber, which can
then be migrated by using the new alter table command.

When publishing stored generated columns instead, it is not possible to
have the same column be stored generated on both the publisher and the
subscriber (see Table 29.2 in section 29.6. Generated Column
Replication). The only supported configuration has a regular column on
the side of the subscriber. (Note that this is not specific to this new
command.)

This makes this scenario a lot easier: the column is migrated on the
publisher only, and the subscriber won't lose any value.

To test these two scenarios, I wrote TAP tests for the subscription
suite. However, I'm inclined to not add them to the test suite. I have
attached them to this email separately.

Other changes since v4
----------------------

I have changed phase 2 to be ran at AT_PASS_ADD_OTHERCONSTR, before it
was at AT_PASS_SET_EXPRESSION. The reason to do it there was to reuse
the cleanup steps in ATPostAlterTypeCleanup when a table rewrite did
happen. But since now never rewrite, this is not necessary anymore.

Looking forward to your thoughts on this!

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

v5 had a rather brittle test that was relying on a DEBUG message being
emitted when phase 3 verifies or rewrites a table (via
client_min_messages). That was failing on the CI because the output
depends on any other setting that might affect logging (in this case it
was log_statement).

The attached v6 replaces it with an injection_point test. If adding new
injection points to detect scans/rewrites is considered too much, I can
back it out and set/reset log_statement too. But I do prefer the
injection_point test.

If the new injection point is good, there are also a couple more tests
which might be updated to use it.

Regards,

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Fri, 2026-07-03 at 08:42 +0200, Alberto Piai wrote:
> In the attached v5 patch I've implemented this design, and went one step
> further (let me know what you think). While discussing this with my
> colleagues at work, the question came up (thanks, Philip!): now that we
> mention the constraint explicitly, what's the point of repeating the
> expression too? The constraint already defines an equality to an
> expression. I think this is a very good point, and it removes one
> further way in which the operation could fail, so I went ahead and
> changed the command to not mention the expression. It takes the
> expression defined in the constraint and uses _that_ as the generator
> expression of the column.

I agree with that idea, it shortend the syntax and leaves less room for
mistakes.

The syntax you ended up with (ADD GENERATED ALWAYS STORED USING CONSTRAINT)
is ugly as hell. I see your point in having ALWAYS and STORED, but perhaps
ADD GENERATED ALWAYS USING CONSTRAINT ... STORED would be better, as it is
syntactically more like GENERATED ALWAYS AS (...) STORED, which would make
it easier to remember.

Or perhaps ADD GENERATED USING CONSTRAINT would be enough, since ALWAYS and
STORED are the only possible choice anyway. I am a bit uncertain on what
is best here.

> > I feel that automatically dropping the constraint is a bit too much
> > black magic, but it is more a feeling than a conviction.
>
> I don't have a strong opinion on whether to cleanup or not, I'll gladly
> take your input. This version of the patch does not drop the constraint
> anymore.

I like it that way, because ALTER TABLE ATTACH PARTITION and ALTER TABLE
ALTER COLUMN SET NOT NULL don't drop the constraint they use either.
But the case is not exactly the same, so I won't insist.

> This version addresses your inputs from the last review:
>
> - I added documentation for the new alter table form to alter_table.sgml
> - Tab completion for psql is there now
> - The missing error conditions in case of an identity column or
> sequences are now handled, more about this in the next section.

Thanks.

I think you didn't adapt the documentation sufficiently after you dropped
the generation expression from the syntax:

> + This form changes a regular column into a stored generated column, using
> + the expression from the given constraint. The constraint must be a
> + <literal>CHECK</literal> constraint proving that the values of the
> + column already satisfy the generation expression. The operation will
> + then be performed without rewriting the table.

The "generation expression" suddenly surfaces towards the end of the paragraph
and makes the reader wonder where it comes from.

Perhaps:

This form changes a regular column into a stored generated column, using
the expression from the given check constraint as generation expression.
The operation will be performed without rewriting the table, which avoids
holding an <literal>ACCESS EXCLUSIVE</literal> lock for a longer time.

That would render the immediately following paragraph unnecessary.

The patch applies, builds and passes the regression tests.

There is a weird asymmetry in that the order in which you write the check
constraint matters:

CREATE TABLE tab (a integer PRIMARY KEY, b integer NOT NULL);
INSERT INTO tab VALUES (1, 2);

-- this fails
ALTER TABLE tab ADD CONSTRAINT c CHECK (2 * a = b);
ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT c;
ERROR: cannot convert a column into a stored generated column without a constraint to prove that the values are consistent
DETAIL: could not find a valid constraint "c" CHECK ("b" = expr) or CHECK ("b" IS NOT DISTINCT FROM (expr))

-- but this works
ALTER TABLE tab DROP CONSTRAINT c;
ALTER TABLE tab ADD CONSTRAINT c CHECK (b = 2 * a);
ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT c;

I think that both variants should be accepted, but I am not certain.

The following error message is not very helpful:

CREATE TABLE tab (
a integer DEFAULT 2,
b integer
CONSTRAINT con CHECK (b IS NOT DISTINCT FROM 2 + random())
);

ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT con;
ERROR: cannot convert a column into a stored generated column without a constraint to prove that the values are consistent
DETAIL: could not find a valid constraint "con" CHECK ("b" IS NOT DISTINCT FROM (expr))

Perhaps it would be better to proceed in three steps:

- find a check constraint with the given name
- verify that the check constraint has the correct shape
- verify that the expression is immutable

Each step could have a different, helpful error message.

I looked at the code too, and I could only spot smaller problems:

> --- a/src/backend/commands/tablecmds.c
> +++ b/src/backend/commands/tablecmds.c
> [...]
> +ATPrepAddExpressionStored(Relation rel,
> + AlterTableCmd *cmd,
> + bool recurse, bool recursing,
> + LOCKMODE lockmode)
> [...]
> + /*
> + * Cannot change only inherited columns to be stored generated columns.
> + */
> + if (!recursing)
> + {
> [...]
> + if (attTup->attinhcount > 0)
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
> + errmsg("cannot change inherited column to be a stored generated column")));
> + }

The comment is cryptic, and I had to read the code to understand what you mean.
Perhaps:

/* convert inherited columns only if the entire hierarchy is changed */

> +/*
> + * Detect dependencies which should stop us from turning a regular column
> + * into a stored generated column.
> + */
> +static void
> +checkDependenciesForAddExprStored(Relation rel,
> + AttrNumber attnum,
> + const char *colName)
> [...]
> + switch (foundObject.classId)
> + {
> + case RelationRelationId:
> + {
> + char relKind = get_rel_relkind(foundObject.objectId);
> +
> + if (relKind == RELKIND_SEQUENCE)
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("cannot convert a serial column to a stored generated column"),
> + errdetail("\"%s\" of relation \"%s\" depends on sequence %s",
> + colName, RelationGetRelationName(rel),
> + getObjectDescription(&foundObject, false))));

There is an extra space in the error message.

> + break;
> + }
> + case AttrDefaultRelationId:
> + {
> + ObjectAddress col = GetAttrDefaultColumnAddress(foundObject.objectId);
> +
> + if (col.objectId == RelationGetRelid(rel) &&
> + col.objectSubId == attnum)
> + {
> + /*
> + * Ignore the column's own default expression. We
> + * handle sequences above, and for a column which is
> + * already a generated column we should never get
> + * here.
> + */

It's not strictly required, but it would be great if you could run pgindent
to get comments and other parts of the code formatted properly.

> + }
> + else
> + {
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("cannot convert a column referenced in a default expression to a stored generated column"),
> + errdetail("Column \"%s\" is referenced by generated column \"%s\".",
> + colName,
> + get_attname(col.objectId, col.objectSubId, false))));

This is confusing me.
The error message seems to suggest that you cannot use a generated column in a
DEFAULT expression. But you can never use other columns in a default expression
anyway, right?
The detail message says something different, namely that the column is referenced
my a generated column (do you mean it is used in the generation expression)?

I believe that if I get confused by the error message, the average user will
also get confused.

> + }
> + break;
> + }
> + default:
> + /* We're not interested in the row */
> + break;
> + }

Perhaps a better comment would be

/* other dependencies, e.g. by views, are no problem */

> +/*
> + * Subroutine for ATExecAddExpressionStored, used to find a CHECK constraint
> + * to prove that the column values statisfy what will be the generator
> + * expression.
> + *
> [...]
> + *
> + * If a valid constraint is found, this returns both the Oid of the constraint
> + * and the unpacked expression.
> + */
> +static Node *
> +findUsableConstraintForAddExprStored(Relation rel, AttrNumber attnum,
> + bool attisnotnull,
> + const char *conname)

I see that it returns a Node, not an Oid and an expression.
If the Oid of the constraint is actually returned somewhere inside the
"Node", the comment should be more specific about it.

> +static ObjectAddress
> +ATExecAddExpressionStored(AlteredTableInfo *tab,
> + Relation rel,
> + const char *colName,
> + Constraint *def)
> +{
> [...]
> + if (attTup->attidentity)
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("Cannot convert an identity column to a stored generated column"),
> + errdetail("column \"%s\" of relation \"%s\" is an identity column",
> + colName, RelationGetRelationName(rel))));

Message style: the main error message should start with a lower case character,
and the detail message should be a whole sentence (initial capitalization, period).
Error messages should try not to exceed 80 characters (no problem with that here),

> + if (has_partition_attrs(rel, colRefs, &is_expr))
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("cannot convert a column into a stored generated column if it's referenced by a partition key"),
> + errdetail("column \"%s\" is part of the partition key of relation \"%s\"",
> + colName, RelationGetRelationName(rel))));

Same as above, plus the error message is too long.
Perhaps:

ERROR: cannot convert a column used in a partitioning key to a generated column

I don't think we need to say "stored" everywhere.

> + if (foundConstraintExpr == NULL)
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("cannot convert a column into a stored generated column without a constraint to prove that the values are consistent"),
> + attTup->attnotnull ?
> + errdetail("could not find a valid constraint \"%s\" CHECK (\"%s\" = expr) or CHECK (\"%s\" IS NOT DISTINCT FROM (expr))",
> + def->conname,
> + colName,
> + colName) :
> + errdetail("could not find a valid constraint \"%s\" CHECK (\"%s\" IS NOT DISTINCT FROM (expr))",
> + def->conname,
> + colName)));

Again, the message must be shorter, and perhaps a hint would be better than a detail:

ERROR: cannot find a check constraint to prove that the column values are correct
HINT: The constraint must be CHECK ("%s" = expr) or CHECK ("%s" IS NOT DISTINCT FROM expr).

I talked about this error message in the beginning. It is thrown whenever
findUsableConstraintForAddExprStored() returns nothing, which is a bit too unspecific.

Perhaps you could have findUsableConstraintForAddExprStored() throw the errors
instead, then they could be more specific and pertinent.

I don't usually mention that, but since you are a new contributor and explicitly
asked several people for a review (which is fine!): it is expected that you also
review other's patches in the commitfest.

Yours,
Laurenz Albe

Hi Laurenz,

thanks a lot for another thoughtful review, I appreciate it a lot. Let
me address the most important point first.

On Tue Jul 7, 2026 at 8:16 PM CEST, Laurenz Albe wrote:
> I don't usually mention that, but since you are a new contributor and explicitly
> asked several people for a review (which is fine!): it is expected that you also
> review other's patches in the commitfest.

You are right to bring that up, it is clearly spelled out in the
developer docs on the wiki and I agree that it's the best (the only?)
way to keep a project going with this volume of contributions.

Until now, I have dedicated all my bandwidth to learning as much as I
could to get this patch to a presentable state. From now on, I'll do my
best to get involved with patch reviews!

>> In the attached v5 patch I've implemented this design, and went one step
>> further (let me know what you think). While discussing this with my
>> colleagues at work, the question came up (thanks, Philip!): now that we
>> mention the constraint explicitly, what's the point of repeating the
>> expression too? The constraint already defines an equality to an
>> expression. I think this is a very good point, and it removes one
>> further way in which the operation could fail, so I went ahead and
>> changed the command to not mention the expression. It takes the
>> expression defined in the constraint and uses _that_ as the generator
>> expression of the column.
>
> I agree with that idea, it shortend the syntax and leaves less room for
> mistakes.
>
> The syntax you ended up with (ADD GENERATED ALWAYS STORED USING CONSTRAINT)
> is ugly as hell. I see your point in having ALWAYS and STORED, but perhaps
> ADD GENERATED ALWAYS USING CONSTRAINT ... STORED would be better, as it is
> syntactically more like GENERATED ALWAYS AS (...) STORED, which would make
> it easier to remember.
>
> Or perhaps ADD GENERATED USING CONSTRAINT would be enough, since ALWAYS and
> STORED are the only possible choice anyway. I am a bit uncertain on what
> is best here.

Your point about moving STORED at the end is good, I feel bad about
dropping it completely though, because in the ADD COLUMN subcommand the
default is VIRTUAL when omitted. Maybe we could drop the ALWAYS, since
that is only relevant when talking about identity columns. That would
result in:

... ADD GENERATED USING CONSTRAINT constr_name STORED

Another option would be to be consistent with SET|DROP EXPRESSION:

... ADD EXPRESSION USING CONSTRAINT constr_name STORED

I would not omit STORED here either, because SET|DROP EXPRESSION act on
what's already a generated column (so they don't need to specify the
type at all), but this one does need to know.

The ALTER TABLE page of the manual already groups these 3 together:

ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY
SET GENERATED { ALWAYS | BY DEFAULT }
DROP IDENTITY [ IF EXISTS ]

These forms change whether a column is an identity column...

Using EXPRESSION for this new command would result in a similar group:

ADD EXPRESSION USING CONSTRAINT constr_name STORED
SET EXPRESSION AS
DROP EXPRESSION [ IF EXISTS ]

the three forms change whether a column is a generated column or change
the generator expression. The first and the third only work with stored
generated columns.

But at this point we are just debating whether to say GENERATED or
EXPRESSION. Personally I find both OK with a slight preference for the
latter.

>> > I feel that automatically dropping the constraint is a bit too much
>> > black magic, but it is more a feeling than a conviction.
>>
>> I don't have a strong opinion on whether to cleanup or not, I'll gladly
>> take your input. This version of the patch does not drop the constraint
>> anymore.
>
> I like it that way, because ALTER TABLE ATTACH PARTITION and ALTER TABLE
> ALTER COLUMN SET NOT NULL don't drop the constraint they use either.
> But the case is not exactly the same, so I won't insist.

It's a valid point, I would leave it like this if there are no further
objections.

All your feedback about docs, error messages and comment is valid too, I
will gladly adapt in the next iteration.

> There is a weird asymmetry in that the order in which you write the check
> constraint matters:
>
> CREATE TABLE tab (a integer PRIMARY KEY, b integer NOT NULL);
> INSERT INTO tab VALUES (1, 2);
>
> -- this fails
> ALTER TABLE tab ADD CONSTRAINT c CHECK (2 * a = b);
> ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT c;
> ERROR: cannot convert a column into a stored generated column without a constraint to prove that the values are consistent
> DETAIL: could not find a valid constraint "c" CHECK ("b" = expr) or CHECK ("b" IS NOT DISTINCT FROM (expr))
>
> -- but this works
> ALTER TABLE tab DROP CONSTRAINT c;
> ALTER TABLE tab ADD CONSTRAINT c CHECK (b = 2 * a);
> ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT c;
>
> I think that both variants should be accepted, but I am not certain.

My original approach here was to be as literal as possible with the
shape of the constraint required to perform this operation. It is after
all an extremely ad-hoc constraint, added just for the purpose of doing
this online migration. But just checking both directions for the
equalities seems simple enough, I'll try to do this in the next version
of the patch.

>> + }
>> + case AttrDefaultRelationId:
>> + {
>> + ObjectAddress col = GetAttrDefaultColumnAddress(foundObject.objectId);
>> +
>> + if (col.objectId == RelationGetRelid(rel) &&
>> + col.objectSubId == attnum)
>> + {
>> + /*
>> + * Ignore the column's own default expression. We
>> + * handle sequences above, and for a column which is
>> + * already a generated column we should never get
>> + * here.
>> + */
>
> It's not strictly required, but it would be great if you could run pgindent
> to get comments and other parts of the code formatted properly.

This is strange, I did run pgindent. Which part seems formatted
incorrectly to you? I'll try to see if we're hitting some edge case that
makes pgindent skip the code.

>> + * If a valid constraint is found, this returns both the Oid of the constraint
>> + * and the unpacked expression.
>> + */
>> +static Node *
>> +findUsableConstraintForAddExprStored(Relation rel, AttrNumber attnum,
>> + bool attisnotnull,
>> + const char *conname)
>
> I see that it returns a Node, not an Oid and an expression.
> If the Oid of the constraint is actually returned somewhere inside the
> "Node", the comment should be more specific about it.

Oops, this is a leftover from when I still returned the Oid of the
constraint to be able to drop it later. Will fix.

Thanks again for your feedback!

Regards,

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Fri, 2026-07-10 at 02:10 +0200, Alberto Piai wrote:
> > The syntax you ended up with (ADD GENERATED ALWAYS STORED USING CONSTRAINT)
> > is ugly as hell. I see your point in having ALWAYS and STORED, but perhaps
> > ADD GENERATED ALWAYS USING CONSTRAINT ... STORED would be better, as it is
> > syntactically more like GENERATED ALWAYS AS (...) STORED, which would make
> > it easier to remember.
> >
> > Or perhaps ADD GENERATED USING CONSTRAINT would be enough, since ALWAYS and
> > STORED are the only possible choice anyway. I am a bit uncertain on what
> > is best here.
>
> Your point about moving STORED at the end is good, I feel bad about
> dropping it completely though, because in the ADD COLUMN subcommand the
> default is VIRTUAL when omitted. Maybe we could drop the ALWAYS, since
> that is only relevant when talking about identity columns. That would
> result in:
>
> ... ADD GENERATED USING CONSTRAINT constr_name STORED
>
> Another option would be to be consistent with SET|DROP EXPRESSION:
>
> ... ADD EXPRESSION USING CONSTRAINT constr_name STORED
>
> I would not omit STORED here either, because SET|DROP EXPRESSION act on
> what's already a generated column (so they don't need to specify the
> type at all), but this one does need to know.
>
> The ALTER TABLE page of the manual already groups these 3 together:
>
> ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY
> SET GENERATED { ALWAYS | BY DEFAULT }
> DROP IDENTITY [ IF EXISTS ]
>
> These forms change whether a column is an identity column...
>
> Using EXPRESSION for this new command would result in a similar group:
>
> ADD EXPRESSION USING CONSTRAINT constr_name STORED
> SET EXPRESSION AS
> DROP EXPRESSION [ IF EXISTS ]
>
> the three forms change whether a column is a generated column or change
> the generator expression. The first and the third only work with stored
> generated columns.
>
> But at this point we are just debating whether to say GENERATED or
> EXPRESSION. Personally I find both OK with a slight preference for the
> latter.

I take your point about keeping the STORED. If VIRTUAL is the default
when creating a generated column, then keeping the explicit STORED makes
sense.

About GENERATED vs. EXPRESSION: my feelings go the other way. If I read

ALTER TABLE tab ALTER col ADD EXPRESSION ...

then it is not clear to me *what* expression is added. After all, there
are not only generation expressions, but also column default expressions,
and I think it is good to make clear that this is about generated columns.

I think I understand your preference: after all, we are not ADDing a
GENERATED column, but only an EXPRESSION that turns a regular column
into a generated column. Perhaps I can convince you by showing the
similar

ALTER TABLE tab ALTER col ADD GENERATED ALWAYS AS IDENTITY;

Here, too, we are not adding a new generated column, only turning a
regular column into a generated one. I think it is good to stay as
close to that already established syntax as possible.

> > >
> > There is a weird asymmetry in that the order in which you write the check
> > constraint matters:
> >
> > CREATE TABLE tab (a integer PRIMARY KEY, b integer NOT NULL);
> > INSERT INTO tab VALUES (1, 2);
> >
> > -- this fails
> > ALTER TABLE tab ADD CONSTRAINT c CHECK (2 * a = b);
> > ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT c;
> > ERROR: cannot convert a column into a stored generated column without a constraint to prove that the values are consistent
> > DETAIL: could not find a valid constraint "c" CHECK ("b" = expr) or CHECK ("b" IS NOT DISTINCT FROM (expr))
> >
> > -- but this works
> > ALTER TABLE tab DROP CONSTRAINT c;
> > ALTER TABLE tab ADD CONSTRAINT c CHECK (b = 2 * a);
> > ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT c;
> >
> > I think that both variants should be accepted, but I am not certain.
>
> My original approach here was to be as literal as possible with the
> shape of the constraint required to perform this operation. It is after
> all an extremely ad-hoc constraint, added just for the purpose of doing
> this online migration. But just checking both directions for the
> equalities seems simple enough, I'll try to do this in the next version
> of the patch.

Right; I thought that it was no big deal. And this would be the only case
in the PostgreSQL SQL dialect that I know where the order of the operands
for an equality condition matters.

> > > + /*
> > > + * Ignore the column's own default expression. We
> > > + * handle sequences above, and for a column which is
> > > + * already a generated column we should never get
> > > + * here.
> > > + */
> >
> > It's not strictly required, but it would be great if you could run pgindent
> > to get comments and other parts of the code formatted properly.
>
> This is strange, I did run pgindent. Which part seems formatted
> incorrectly to you? I'll try to see if we're hitting some edge case that
> makes pgindent skip the code.

I had not tested it, but the lines seemed to be shorter than the 80 columns
to which pgindent formats comments.
If you already ran pgindent, please ignore my comment.

Yours,
Laurenz Albe

Hi,

On Fri Jul 10, 2026 at 8:51 AM CEST, Laurenz Albe wrote:
> I take your point about keeping the STORED. If VIRTUAL is the default
> when creating a generated column, then keeping the explicit STORED makes
> sense.
>
> About GENERATED vs. EXPRESSION: my feelings go the other way. If I read
>
> ALTER TABLE tab ALTER col ADD EXPRESSION ...
>
> then it is not clear to me *what* expression is added. After all, there
> are not only generation expressions, but also column default expressions,
> and I think it is good to make clear that this is about generated columns.
>
> I think I understand your preference: after all, we are not ADDing a
> GENERATED column, but only an EXPRESSION that turns a regular column
> into a generated column. Perhaps I can convince you by showing the
> similar
>
> ALTER TABLE tab ALTER col ADD GENERATED ALWAYS AS IDENTITY;
>
> Here, too, we are not adding a new generated column, only turning a
> regular column into a generated one. I think it is good to stay as
> close to that already established syntax as possible.

Yes that is convincing.

PFA v7, implementing this version of the command:

... ALTER col ADD GENERATED USING CONSTRAINT constr_name STORED

Differences from v6, besides the new syntax:

- both the = and the not-distinct form of the constraints now work with
both orders of the operands

- comments and error messages changed to address the feedback from the
latest review round

- simplified tests a bit, trying to avoid creating/dropping test tables
unnecessarily

- ATPrepAddGenStore doesn't try to lock child tables anymore, following
what was done for DROP EXPRESSION in
fef160d8b0ddf72e1089133300d30109293f5a71 [0]

In particular, all the error messages now follow the guidelines for
error reporting. I tried to use a consistent error message everywhere,
adding details and hints where appropriate.

All the error cases are grouped together now in the regress suite, so
looking at the expected output should give a good overview of all the
error conditions and how they are reported to the user.

A note about this one:

> The following error message is not very helpful:
>
>
> CREATE TABLE tab (
> a integer DEFAULT 2,
> b integer
> CONSTRAINT con CHECK (b IS NOT DISTINCT FROM 2 + random())
> );
>
>
> ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT con;
> ERROR: cannot convert a column into a stored generated column without a constraint to prove that the values are consistent
> DETAIL: could not find a valid constraint "con" CHECK ("b" IS NOT DISTINCT FROM (expr))

This was interesting. What's going on here is that since random()
returns a float, the whole expression returns a float. The column b is
an int, so since there is an implicit cast from int to float, the
resulting expression for the CHECK constraint is

b::float IS NOT DISTINCT FROM 2 + random()

I think in cases like this there's not much I can do: the constraint
isn't an equality to b anymore, but an equality to f(b) where f is
the function defined for the cast. The constraint is simply not usable
for our purpose.

For this reason, I think it doesn't make too much sense in this case to
look at the other operand, hunt down the random() and complain about the
function being volatile: the core problem here is the return type, and
the same situation can happen with an immutable function.

I tried detecting implicit casts though, because I think this is a
mistake that's quite easy to make, so it's worth trying to give a hint
to the user about what's going on and what to do.

This is now reported in this way (from the regress test suite):

alter table tgen.t1 add constraint chk_gen check (b is not distinct from (a + random()));
-- the hint should inform about the type cast
alter table tgen.t1 alter column b add generated using constraint chk_gen stored;
ERROR: cannot convert column "b" to generated
DETAIL: Could not find a valid constraint "chk_gen" CHECK ("b" IS NOT DISTINCT FROM expr).
HINT: Ensure that the type of the expression matches the type of the column.

In this situation, \d would show the constraint being

CHECK (b::double precision = (.... expr with random())

instead of b = ...expr, which makes me think the hint is clear enough.
But I'm curious to hear what you think about it.

Independently from the problem with casts, the immutability of the
generation expression is of course also checked:

alter table tgen.t2 add constraint chk_gen check (b is not distinct from (a + random()::int));
alter table tgen.t2 alter column b
add generated using constraint chk_gen stored;
ERROR: generation expression is not immutable

Looking forward to your comments!

Regards,

Alberto

[0] https://www.postgresql.org/message-id/anGRnFgKM6RFmGLm%40alvherre.pgsql

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Thu, 2026-08-06 at 14:36 +0200, Alberto Piai wrote:
> PFA v7, implementing this version of the command:
>
> ... ALTER col ADD GENERATED USING CONSTRAINT constr_name STORED

Great! I think this is pretty much good to go.

> In particular, all the error messages now follow the guidelines for
> error reporting. I tried to use a consistent error message everywhere,
> adding details and hints where appropriate.

Much better!

I was wondering about this hint:

Use this command on the root table/partition without ONLY.

You can never turn a column of a partition into a generated column, right?
How about

Use the command on the partition root without specifying ONLY.

This detail message is longer than 80 characters:

Converting a column to a stored generated column can only be done on the whole hierarchy at once.

How about

Converting only part of a partitioning/inheritance hierarchy is not supported.

> A note about this one:
>
> > The following error message is not very helpful:
> >
> >
> > CREATE TABLE tab (
> > a integer DEFAULT 2,
> > b integer
> > CONSTRAINT con CHECK (b IS NOT DISTINCT FROM 2 + random())
> > );
> >
> >
> > ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT con;
> > ERROR: cannot convert a column into a stored generated column without a constraint to prove that the values are consistent
> > DETAIL: could not find a valid constraint "con" CHECK ("b" IS NOT DISTINCT FROM (expr))
>
> This was interesting. What's going on here is that since random()
> returns a float, the whole expression returns a float. The column b is
> an int, so since there is an implicit cast from int to float, the
> resulting expression for the CHECK constraint is
>
> b::float IS NOT DISTINCT FROM 2 + random()
>
> I think in cases like this there's not much I can do: the constraint
> isn't an equality to b anymore, but an equality to f(b) where f is
> the function defined for the cast. The constraint is simply not usable
> for our purpose.
>
> For this reason, I think it doesn't make too much sense in this case to
> look at the other operand, hunt down the random() and complain about the
> function being volatile: the core problem here is the return type, and
> the same situation can happen with an immutable function.
>
> I tried detecting implicit casts though, because I think this is a
> mistake that's quite easy to make, so it's worth trying to give a hint
> to the user about what's going on and what to do.
>
> This is now reported in this way (from the regress test suite):
>
> alter table tgen.t1 add constraint chk_gen check (b is not distinct from (a + random()));
> -- the hint should inform about the type cast
> alter table tgen.t1 alter column b add generated using constraint chk_gen stored;
> ERROR: cannot convert column "b" to generated
> DETAIL: Could not find a valid constraint "chk_gen" CHECK ("b" IS NOT DISTINCT FROM expr).
> HINT: Ensure that the type of the expression matches the type of the column.
>
> In this situation, \d would show the constraint being
>
> CHECK (b::double precision = (.... expr with random())
>
> instead of b = ...expr, which makes me think the hint is clear enough.
> But I'm curious to hear what you think about it.
>
> Independently from the problem with casts, the immutability of the
> generation expression is of course also checked:
>
> alter table tgen.t2 add constraint chk_gen check (b is not distinct from (a + random()::int));
> alter table tgen.t2 alter column b
> add generated using constraint chk_gen stored;
> ERROR: generation expression is not immutable

I agree with your assessment; thanks for the additional hint!

There is one sentence in the documentation that sounds wrong to me:

+ <para>
+ After this command is run, <literal>column_name</literal> will be a stored
+ generated column with <literal>expr</literal> as its generation
+ expression.
+ </para>

Shouldn't it be "after this command has been run"? Or perhaps "has completed"?

Yours,
Laurenz Albe

Hi Laurenz,

thanks for your feedback!

On Mon Aug 24, 2026 at 7:29 PM CEST, Laurenz Albe wrote:
> On Thu, 2026-08-06 at 14:36 +0200, Alberto Piai wrote:
>> PFA v7, implementing this version of the command:
>>
>> ... ALTER col ADD GENERATED USING CONSTRAINT constr_name STORED
>
> Great! I think this is pretty much good to go.
>
>> In particular, all the error messages now follow the guidelines for
>> error reporting. I tried to use a consistent error message everywhere,
>> adding details and hints where appropriate.
>
> Much better!
>
> I was wondering about this hint:
>
> Use this command on the root table/partition without ONLY.
>
> You can never turn a column of a partition into a generated column, right?
> How about
>
> Use the command on the partition root without specifying ONLY.

Since this is also taking care of classic inheritance, it seems strange
to only mention partitions. But I see that this way it's confusing (and
I do concede that partitioning might be the more widely used of the two)
so I switched it around to "on the root partition/table without
specifying ONLY". Does that make more sense?

> This detail message is longer than 80 characters:
>
> Converting a column to a stored generated column can only be done on the whole hierarchy at once.
>
> How about
>
> Converting only part of a partitioning/inheritance hierarchy is not supported.

Much better, fixed.

> There is one sentence in the documentation that sounds wrong to me:
>
> + <para>
> + After this command is run, <literal>column_name</literal> will be a stored
> + generated column with <literal>expr</literal> as its generation
> + expression.
> + </para>
>
> Shouldn't it be "after this command has been run"? Or perhaps "has completed"?

Thanks, changed to "has completed".

The attached v8 addresses the points above.

Kind regards,

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Mon, 2026-08-24 at 22:03 +0200, Alberto Piai wrote:
> > I was wondering about this hint:
> >
> >    Use this command on the root table/partition without ONLY.
> >
> > You can never turn a column of a partition into a generated column, right?
> > How about
> >
> >    Use the command on the partition root without specifying ONLY.
>
> Since this is also taking care of classic inheritance, it seems strange
> to only mention partitions. But I see that this way it's confusing (and
> I do concede that partitioning might be the more widely used of the two)
> so I switched it around to "on the root partition/table without
> specifying ONLY". Does that make more sense?

My problem is that there is no "root partition".

Perhaps

Use the command on the partitioning or inheritance root without specifying ONLY.

> > This detail message is longer than 80 characters:
> >
> >    Converting a column to a stored generated column can only be done on the whole hierarchy at once.
> >
> > How about
> >
> >    Converting only part of a partitioning/inheritance hierarchy is not supported.
>
> Much better, fixed.

I am having second thoughts about the slash.
Perhaps it would be better to use "or" instead.

Yours,
Laurenz Albe

On Tue Aug 25, 2026 at 10:46 AM CEST, Laurenz Albe wrote:
> On Mon, 2026-08-24 at 22:03 +0200, Alberto Piai wrote:
>> > I was wondering about this hint:
>> >
>> >    Use this command on the root table/partition without ONLY.
>> >
>> > You can never turn a column of a partition into a generated column, right?
>> > How about
>> >
>> >    Use the command on the partition root without specifying ONLY.
>>
>> Since this is also taking care of classic inheritance, it seems strange
>> to only mention partitions. But I see that this way it's confusing (and
>> I do concede that partitioning might be the more widely used of the two)
>> so I switched it around to "on the root partition/table without
>> specifying ONLY". Does that make more sense?
>
> My problem is that there is no "root partition".

Ah, I see what you mean now. I agree, "root partition" makes no sense.

So, removing the slash, it would look like:

ERROR: cannot convert column "c" to generated
DETAIL: Converting only part of a partitioning or inheritance hierarchy is not supported.
HINT: Use this command on the partitioning or inheritance root without specifying ONLY.

Maybe a bit heavy on the eye?

Since the root of the hierarchy is in both cases just a table
(partitoned or not), what about just calling it a table?

ERROR: cannot convert column "c" to generated
DETAIL: Converting only part of a partitioning or inheritance hierarchy is not supported.
HINT: Use this command on the root table without specifying ONLY.

Regards,

Alberto

--
Alberto Piai
Sensational AG
Zürich, Switzerland

On Tue, 2026-08-25 at 18:13 +0200, Alberto Piai wrote:
> Ah, I see what you mean now. I agree, "root partition" makes no sense.
>
> So, removing the slash, it would look like:
>
>   ERROR:  cannot convert column "c" to generated
>   DETAIL:  Converting only part of a partitioning or inheritance hierarchy is not supported.
>   HINT:  Use this command on the partitioning or inheritance root without specifying ONLY.
>
> Maybe a bit heavy on the eye?
>
> Since the root of the hierarchy is in both cases just a table
> (partitoned or not), what about just calling it a table?
>
>   ERROR:  cannot convert column "c" to generated
>   DETAIL:  Converting only part of a partitioning or inheritance hierarchy is not supported.
>   HINT:  Use this command on the root table without specifying ONLY.

Both variants look fine to me. Pick the one you prefer.

Yours,
Laurenz Albe