Message-ID: <79b63e772179abb4f5ad749d9e067d48a5537599.camel@cybertec.at>
Subject: Re: Adding a stored generated column without long-lived locks
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Alberto Piai <alberto.piai@gmail.com>, pgsql-hackers@postgresql.org
Cc: =?ISO-8859-1?Q?=C1lvaro?= Herrera <alvherre@kurilemu.de>
Date: Fri, 10 Jul 2026 08:51:58 +0200
In-Reply-To: <DJUGET7QLNRK.7G2H4M2PPJM9@gmail.com>
References: <abkrpUwlGngF4e-d@phidippus.sen.work>
	 <DHMSK551GIM8.1B1CN2JN8BK50@gmail.com>
	 <DI19FP3X4VI0.21FO6V1HGJK5P@gmail.com>
	 <DIIRFCWJIA80.2Q5DYIX6C7KZ5@gmail.com>
	 <DITN90YX3328.2A0I3LCV164BA@gmail.com>
	 <3af338791e8ab27c4a7efbc4217c97162de76bc8.camel@cybertec.at>
	 <DJMFASI8OYBS.2JBMQ7KZ2XOTO@gmail.com>
	 <DJOPT622HYJK.3JRH1VQCV5CDL@gmail.com>
	 <2eb68e74ea95fc2f691de7d8cd28448d00d1d216.camel@cybertec.at>
	 <DJUGET7QLNRK.7G2H4M2PPJM9@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.58.3 (3.58.3-1.fc43) 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/79b63e772179abb4f5ad749d9e067d48a5537599.camel%40cybertec.at>
Precedence: bulk

On Fri, 2026-07-10 at 02:10 +0200, Alberto Piai wrote:
> > The syntax you ended up with (ADD GENERATED ALWAYS STORED USING CONSTRA=
INT)
> > is ugly as hell.  I see your point in having ALWAYS and STORED, but per=
haps
> > ADD GENERATED ALWAYS USING CONSTRAINT ... STORED would be better, as it=
 is
> > syntactically more like GENERATED ALWAYS AS (...) STORED, which would m=
ake
> > it easier to remember.
> >=20
> > 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 wh=
at
> > is best here.
>=20
> 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:
>=20
> ... ADD GENERATED USING CONSTRAINT constr_name STORED
>=20
> Another option would be to be consistent with SET|DROP EXPRESSION:
>=20
> ... ADD EXPRESSION USING CONSTRAINT constr_name STORED
>=20
> 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.
>=20
> The ALTER TABLE page of the manual already groups these 3 together:
>=20
>     ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY
>     SET GENERATED { ALWAYS | BY DEFAULT }
>     DROP IDENTITY [ IF EXISTS ]
>=20
>       These forms change whether a column is an identity column...
>=20
> Using EXPRESSION for this new command would result in a similar group:
>=20
>     ADD EXPRESSION USING CONSTRAINT constr_name STORED
>     SET EXPRESSION AS
>     DROP EXPRESSION [ IF EXISTS ]
>=20
> 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.
>=20
> 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.

> > >=20
> > There is a weird asymmetry in that the order in which you write the che=
ck
> > constraint matters:
> >=20
> >   CREATE TABLE tab (a integer PRIMARY KEY, b integer NOT NULL);
> >   INSERT INTO tab VALUES (1, 2);
> >=20
> >   -- this fails
> >   ALTER TABLE tab ADD CONSTRAINT c CHECK (2 * a =3D b);
> >   ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT =
c;
> >   ERROR:  cannot convert a column into a stored generated column withou=
t a constraint to prove that the values are consistent
> >   DETAIL:  could not find a valid constraint "c" CHECK ("b" =3D expr) o=
r CHECK ("b" IS NOT DISTINCT FROM (expr))
> >=20
> >   -- but this works
> >   ALTER TABLE tab DROP CONSTRAINT c;
> >   ALTER TABLE tab ADD CONSTRAINT c CHECK (b =3D 2 * a);
> >   ALTER TABLE tab ALTER b ADD GENERATED ALWAYS STORED USING CONSTRAINT =
c;
> >=20
> > I think that both variants should be accepted, but I am not certain.
>=20
> 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 w=
hich is
> > > +                        * already a generated column we should never=
 get
> > > +                        * here.
> > > +                        */
> >=20
> > It's not strictly required, but it would be great if you could run pgin=
dent
> > to get comments and other parts of the code formatted properly.
>=20
> 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