| From: | Alberto Piai <alberto(dot)piai(at)gmail(dot)com> |
|---|---|
| To: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Alberto Piai <alberto(dot)piai(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
| Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Subject: | Re: Adding a stored generated column without long-lived locks |
| Date: | 2026-08-06 12:36:21 |
| Message-ID: | DKHUPZ08JV6G.2938OV580K72C@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| v7-0001-Support-changing-a-column-into-a-stored-generated.patch | text/plain | 71.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ayush Tiwari | 2026-08-06 12:40:13 | Error handling in after-startup shmem requests |
| Previous Message | Ashutosh Sharma | 2026-08-06 12:26:26 | Re: [PATCH] Release replication slot on error in SQL-callable slot functions |