Horton

Thread

Commits

  1. REPACK CONCURRENTLY: Initialize the range table more honestly

  2. Fix REPACK CONCURRENTLY for stored generated columns

  1. REPACK CONCURRENTLY fails on tables with generated columns

    Ewan Young <kdbase.hack@gmail.com> — 2026-06-12T08:39:54Z

    Hi,

REPACK (CONCURRENTLY) aborts with an internal error on any table that has
a STORED generated column, if a concurrent UPDATE that requires index
maintenance is applied during the catch-up phase:

ERROR:  no generation expression found for column number 3 of table
"pg_temp_16396"
Plain (non-concurrent) REPACK on such a table works fine, and so does
REPACK (CONCURRENTLY) as long as no qualifying concurrent change is
applied -- so the problem is specific to the concurrent-change apply path.

The attached patch adds an isolation test, but here is the manual
sequence (server built with --enable-injection-points):

CREATE EXTENSION injection_points;
CREATE TABLE t (i int PRIMARY KEY, v int,
                g int GENERATED ALWAYS AS (v * 10) STORED);
CREATE INDEX ON t (v);              -- makes UPDATE of v non-HOT
INSERT INTO t(i, v) VALUES (1, 1);

-- session 1:
SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
REPACK (CONCURRENTLY) t;            -- blocks at the injection point

-- session 2, once session 1 is waiting:
UPDATE t SET v = v + 1 WHERE i = 1;
SELECT injection_points_wakeup('repack-concurrently-before-lock');

-- session 1 then fails with the ERROR above.
Without injection points this is a race: the concurrent UPDATE has to be
decoded and applied during catch-up, and it has to be a non-HOT update
(one that goes through index maintenance). It is reliably hit on a busy
table with a generated column.

The transient heap built by make_new_heap() is intentionally created
without the old table's defaults and constraints, so it has no generation
expressions for its generated columns, even though the tuple descriptor
still has attgenerated set.

When apply_concurrent_update() replays a non-HOT update, it calls
ExecInsertIndexTuples() with EIIT_IS_UPDATE. To decide whether to pass
the "indexUnchanged" hint, that calls index_unchanged_by_update() ->
ExecGetExtraUpdatedCols() -> ExecInitGenerated(), which looks up the
generation expression of each generated column via build_column_default()
and errors out when it finds none on the transient heap.

The apply path does not need to recompute generated columns at all: the
decoded tuple already carries the correct value, and it is only inserted.
Note also that ExecGetUpdatedCols() already returns an empty set for this
ResultRelInfo, because it is not part of any range table -- so the
indexUnchanged determination here is already approximate.

Regards,
Ewan Young

  2. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Antonin Houska <ah@cybertec.at> — 2026-06-12T09:52:12Z

    Ewan Young <kdbase.hack@gmail.com> wrote:

> REPACK (CONCURRENTLY) aborts with an internal error on any table that has
> a STORED generated column, if a concurrent UPDATE that requires index
> maintenance is applied during the catch-up phase:
> 
> ERROR:  no generation expression found for column number 3 of table
> "pg_temp_16396"
> Plain (non-concurrent) REPACK on such a table works fine, and so does
> REPACK (CONCURRENTLY) as long as no qualifying concurrent change is
> applied -- so the problem is specific to the concurrent-change apply path.

Thanks for the report!

> The attached patch adds an isolation test, but here is the manual
> sequence (server built with --enable-injection-points):
> 
> CREATE EXTENSION injection_points;
> CREATE TABLE t (i int PRIMARY KEY, v int,
>                 g int GENERATED ALWAYS AS (v * 10) STORED);
> CREATE INDEX ON t (v);              -- makes UPDATE of v non-HOT
> INSERT INTO t(i, v) VALUES (1, 1);
> 
> -- session 1:
> SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
> REPACK (CONCURRENTLY) t;            -- blocks at the injection point
> 
> -- session 2, once session 1 is waiting:
> UPDATE t SET v = v + 1 WHERE i = 1;
> SELECT injection_points_wakeup('repack-concurrently-before-lock');
> 
> -- session 1 then fails with the ERROR above.

I confirm I can reproduce it. I'll post a fix next week.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

  3. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Álvaro Herrera <alvherre@kurilemu.de> — 2026-06-12T10:25:29Z

    On 2026-Jun-12, Antonin Houska wrote:

> I confirm I can reproduce it. I'll post a fix next week.

Didn't you like Ewan's proposed patch?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Update: super-fast reaction on the Postgres bugs mailing list. The report
was acknowledged [...], and a fix is under discussion.
The wonders of open-source !"
             https://twitter.com/gunnarmorling/status/1596080409259003906

  4. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Antonin Houska <ah@cybertec.at> — 2026-06-12T11:01:04Z

    Alvaro Herrera <alvherre@kurilemu.de> wrote:

> On 2026-Jun-12, Antonin Houska wrote:
> 
> > I confirm I can reproduce it. I'll post a fix next week.
> 
> Didn't you like Ewan's proposed patch?

After having read the email, I thought that the attachment only contains the
reproducing test. Now that I look into it, I see it also contains a fix. Sorry
for the confusion.

I cannot concentrate on it today, so if I'm supposed to review it, I need a
few days.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

  5. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Álvaro Herrera <alvherre@kurilemu.de> — 2026-06-12T11:40:30Z

    Hello,

On 2026-Jun-12, Antonin Houska wrote:

> After having read the email, I thought that the attachment only contains the
> reproducing test. Now that I look into it, I see it also contains a fix. Sorry
> for the confusion.
> 
> I cannot concentrate on it today, so if I'm supposed to review it, I need a
> few days.

I would welcome your review, but it's not strictly mandatory.  That
said, we have some weeks before the next beta, so there's no rush.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"After a quick R of TFM, all I can say is HOLY CR** THAT IS COOL! PostgreSQL was
amazing when I first started using it at 7.2, and I'm continually astounded by
learning new features and techniques made available by the continuing work of
the development team."
Berend Tober, http://archives.postgresql.org/pgsql-hackers/2007-08/msg01009.php

  6. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Srinath Reddy Sadipiralla <srinath2133@gmail.com> — 2026-06-14T03:56:48Z

    Hi Ewan,

On Fri, Jun 12, 2026 at 2:10 PM Ewan Young <kdbase.hack@gmail.com> wrote:

> Hi,
>
> REPACK (CONCURRENTLY) aborts with an internal error on any table that has
> a STORED generated column, if a concurrent UPDATE that requires index
> maintenance is applied during the catch-up phase:
>
> ERROR:  no generation expression found for column number 3 of table
> "pg_temp_16396"
> Plain (non-concurrent) REPACK on such a table works fine, and so does
> REPACK (CONCURRENTLY) as long as no qualifying concurrent change is
> applied -- so the problem is specific to the concurrent-change apply path.
>
> The attached patch adds an isolation test, but here is the manual
> sequence (server built with --enable-injection-points):
>
> CREATE EXTENSION injection_points;
> CREATE TABLE t (i int PRIMARY KEY, v int,
>                 g int GENERATED ALWAYS AS (v * 10) STORED);
> CREATE INDEX ON t (v);              -- makes UPDATE of v non-HOT
> INSERT INTO t(i, v) VALUES (1, 1);
>
> -- session 1:
> SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
> REPACK (CONCURRENTLY) t;            -- blocks at the injection point
>
> -- session 2, once session 1 is waiting:
> UPDATE t SET v = v + 1 WHERE i = 1;
> SELECT injection_points_wakeup('repack-concurrently-before-lock');
>
> -- session 1 then fails with the ERROR above.
> Without injection points this is a race: the concurrent UPDATE has to be
> decoded and applied during catch-up, and it has to be a non-HOT update
> (one that goes through index maintenance). It is reliably hit on a busy
> table with a generated column.
>

I was able to reproduce this.


>
> The transient heap built by make_new_heap() is intentionally created
> without the old table's defaults and constraints, so it has no generation
> expressions for its generated columns, even though the tuple descriptor
> still has attgenerated set.
>
> When apply_concurrent_update() replays a non-HOT update, it calls
> ExecInsertIndexTuples() with EIIT_IS_UPDATE. To decide whether to pass
> the "indexUnchanged" hint, that calls index_unchanged_by_update() ->
> ExecGetExtraUpdatedCols() -> ExecInitGenerated(), which looks up the
> generation expression of each generated column via build_column_default()
> and errors out when it finds none on the transient heap.
>
> The apply path does not need to recompute generated columns at all: the
> decoded tuple already carries the correct value, and it is only inserted.
> Note also that ExecGetUpdatedCols() already returns an empty set for this
> ResultRelInfo, because it is not part of any range table -- so the
> indexUnchanged determination here is already approximate.
>

makes sense, i have reviewed the patch, it LGTM.

-- 
Thanks :)
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/

  7. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Ewan Young <kdbase.hack@gmail.com> — 2026-06-16T02:02:15Z

    Hi Srinath,

Thanks a lot for reproducing it and for the careful review.

On Sun, Jun 14, 2026 at 11:57 AM Srinath Reddy Sadipiralla
<srinath2133@gmail.com> wrote:
>
> Hi Ewan,
>
> On Fri, Jun 12, 2026 at 2:10 PM Ewan Young <kdbase.hack@gmail.com> wrote:
>>
>> Hi,
>>
>> REPACK (CONCURRENTLY) aborts with an internal error on any table that has
>> a STORED generated column, if a concurrent UPDATE that requires index
>> maintenance is applied during the catch-up phase:
>>
>> ERROR:  no generation expression found for column number 3 of table
>> "pg_temp_16396"
>> Plain (non-concurrent) REPACK on such a table works fine, and so does
>> REPACK (CONCURRENTLY) as long as no qualifying concurrent change is
>> applied -- so the problem is specific to the concurrent-change apply path.
>>
>> The attached patch adds an isolation test, but here is the manual
>> sequence (server built with --enable-injection-points):
>>
>> CREATE EXTENSION injection_points;
>> CREATE TABLE t (i int PRIMARY KEY, v int,
>>                 g int GENERATED ALWAYS AS (v * 10) STORED);
>> CREATE INDEX ON t (v);              -- makes UPDATE of v non-HOT
>> INSERT INTO t(i, v) VALUES (1, 1);
>>
>> -- session 1:
>> SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
>> REPACK (CONCURRENTLY) t;            -- blocks at the injection point
>>
>> -- session 2, once session 1 is waiting:
>> UPDATE t SET v = v + 1 WHERE i = 1;
>> SELECT injection_points_wakeup('repack-concurrently-before-lock');
>>
>> -- session 1 then fails with the ERROR above.
>> Without injection points this is a race: the concurrent UPDATE has to be
>> decoded and applied during catch-up, and it has to be a non-HOT update
>> (one that goes through index maintenance). It is reliably hit on a busy
>> table with a generated column.
>
>
> I was able to reproduce this.
>
>>
>>
>> The transient heap built by make_new_heap() is intentionally created
>> without the old table's defaults and constraints, so it has no generation
>> expressions for its generated columns, even though the tuple descriptor
>> still has attgenerated set.
>>
>> When apply_concurrent_update() replays a non-HOT update, it calls
>> ExecInsertIndexTuples() with EIIT_IS_UPDATE. To decide whether to pass
>> the "indexUnchanged" hint, that calls index_unchanged_by_update() ->
>> ExecGetExtraUpdatedCols() -> ExecInitGenerated(), which looks up the
>> generation expression of each generated column via build_column_default()
>> and errors out when it finds none on the transient heap.
>>
>> The apply path does not need to recompute generated columns at all: the
>> decoded tuple already carries the correct value, and it is only inserted.
>> Note also that ExecGetUpdatedCols() already returns an empty set for this
>> ResultRelInfo, because it is not part of any range table -- so the
>> indexUnchanged determination here is already approximate.
>
>
> makes sense, i have reviewed the patch, it LGTM.
>
> --
> Thanks :)
> Srinath Reddy Sadipiralla
> EDB: https://www.enterprisedb.com/

  8. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Antonin Houska <ah@cybertec.at> — 2026-06-22T11:12:11Z

    Ewan Young <kdbase.hack@gmail.com> wrote:

> The transient heap built by make_new_heap() is intentionally created
> without the old table's defaults and constraints, so it has no generation
> expressions for its generated columns, even though the tuple descriptor
> still has attgenerated set.
> 
> When apply_concurrent_update() replays a non-HOT update, it calls
> ExecInsertIndexTuples() with EIIT_IS_UPDATE. To decide whether to pass
> the "indexUnchanged" hint, that calls index_unchanged_by_update() ->
> ExecGetExtraUpdatedCols() -> ExecInitGenerated(), which looks up the
> generation expression of each generated column via build_column_default()
> and errors out when it finds none on the transient heap.
> 
> The apply path does not need to recompute generated columns at all: the
> decoded tuple already carries the correct value, and it is only inserted.
> Note also that ExecGetUpdatedCols() already returns an empty set for this
> ResultRelInfo, because it is not part of any range table -- so the
> indexUnchanged determination here is already approximate.

I'm sorry for the confusion, but the fact that ExecGetUpdatedCols() returns an
empty set is an omission rather than deliberate choice. Assuming we fix that,
the result of ExecGetExtraUpdatedCols() does matter. Thus we should copy the
related pg_attrdef entries, as I suggest in this patch.

Another question is how serious problem it is that ExecGetUpdatedCols()
returns empty set. AFAICS, "indexUnchanged" does not affect correctness - it's
is only a hint that helps the btree AM decide whent the bottom-up deletion and
de-duplication techniques should (not) be used. I'm not sure it's easy to
update the set for individual UPDATEs: the UPDATE commands REPACK replays
originate from different SQL queries and the logical decoding does not
transfer this information.

Even then, I think it'd be "less bad" to have ExecGetUpdatedCols() return a
set containing all the attributes rather than empty set. That is, avoid using
the btree optimizations altogether rather than allow them them when not
appropriate. However, per index_unchanged_by_update(), if ExecGetUpdatedCols()
tells that all columns are updated, the result of ExecGetExtraUpdatedCols()
does not matter.  Nevertheless, I'd still slightly prefer copying the
pg_attrdef entries to hacking the executor.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

  9. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Ewan Young <kdbase.hack@gmail.com> — 2026-06-22T12:49:54Z

    Hi Antonin,

On Mon, Jun 22, 2026 at 7:12 PM Antonin Houska <ah@cybertec.at> wrote:
>
> Ewan Young <kdbase.hack@gmail.com> wrote:
>
> > The transient heap built by make_new_heap() is intentionally created
> > without the old table's defaults and constraints, so it has no generation
> > expressions for its generated columns, even though the tuple descriptor
> > still has attgenerated set.
> >
> > When apply_concurrent_update() replays a non-HOT update, it calls
> > ExecInsertIndexTuples() with EIIT_IS_UPDATE. To decide whether to pass
> > the "indexUnchanged" hint, that calls index_unchanged_by_update() ->
> > ExecGetExtraUpdatedCols() -> ExecInitGenerated(), which looks up the
> > generation expression of each generated column via build_column_default()
> > and errors out when it finds none on the transient heap.
> >
> > The apply path does not need to recompute generated columns at all: the
> > decoded tuple already carries the correct value, and it is only inserted.
> > Note also that ExecGetUpdatedCols() already returns an empty set for this
> > ResultRelInfo, because it is not part of any range table -- so the
> > indexUnchanged determination here is already approximate.
>
> I'm sorry for the confusion, but the fact that ExecGetUpdatedCols() returns an
> empty set is an omission rather than deliberate choice. Assuming we fix that,
> the result of ExecGetExtraUpdatedCols() does matter. Thus we should copy the
> related pg_attrdef entries, as I suggest in this patch.
>
> Another question is how serious problem it is that ExecGetUpdatedCols()
> returns empty set. AFAICS, "indexUnchanged" does not affect correctness - it's
> is only a hint that helps the btree AM decide whent the bottom-up deletion and
> de-duplication techniques should (not) be used. I'm not sure it's easy to
> update the set for individual UPDATEs: the UPDATE commands REPACK replays
> originate from different SQL queries and the logical decoding does not
> transfer this information.
>
> Even then, I think it'd be "less bad" to have ExecGetUpdatedCols() return a
> set containing all the attributes rather than empty set. That is, avoid using
> the btree optimizations altogether rather than allow them them when not
> appropriate. However, per index_unchanged_by_update(), if ExecGetUpdatedCols()
> tells that all columns are updated, the result of ExecGetExtraUpdatedCols()
> does not matter.  Nevertheless, I'd still slightly prefer copying the
> pg_attrdef entries to hacking the executor.

Agreed, thanks for the correction. Relying on the empty ExecGetUpdatedCols()
set was the weak point of my version -- it's an omission, not something to
build a second approximation on. Copying the pg_attrdef entries fixes the
inconsistency at the root, so let's go with your approach.

I applied the patch and ran it through an injection-point reproducer
(cassert). Without the fix the bug reproduces (ERROR: no generation
expression found for column number 3 ...); with it, REPACK CONCURRENTLY
succeeds under a concurrent non-HOT UPDATE for a STORED generated column, an
index directly on the generated column, and a VIRTUAL column, with correct
values afterwards. Your repack.spec change passes.

The approach is right and I've confirmed it fixes the bug, so +1 from me in
this direction.

>
> --
> Antonin Houska
> Web: https://www.cybertec-postgresql.com
>

Regards,
Ewan Young

  10. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Antonin Houska <ah@cybertec.at> — 2026-07-01T13:57:02Z

    Ewan Young <kdbase.hack@gmail.com> wrote:

> On Mon, Jun 22, 2026 at 7:12 PM Antonin Houska <ah@cybertec.at> wrote:
> >
> > Ewan Young <kdbase.hack@gmail.com> wrote:
> >
> > > The transient heap built by make_new_heap() is intentionally created
> > > without the old table's defaults and constraints, so it has no generation
> > > expressions for its generated columns, even though the tuple descriptor
> > > still has attgenerated set.
> > >
> > > When apply_concurrent_update() replays a non-HOT update, it calls
> > > ExecInsertIndexTuples() with EIIT_IS_UPDATE. To decide whether to pass
> > > the "indexUnchanged" hint, that calls index_unchanged_by_update() ->
> > > ExecGetExtraUpdatedCols() -> ExecInitGenerated(), which looks up the
> > > generation expression of each generated column via build_column_default()
> > > and errors out when it finds none on the transient heap.
> > >
> > > The apply path does not need to recompute generated columns at all: the
> > > decoded tuple already carries the correct value, and it is only inserted.
> > > Note also that ExecGetUpdatedCols() already returns an empty set for this
> > > ResultRelInfo, because it is not part of any range table -- so the
> > > indexUnchanged determination here is already approximate.
> >
> > I'm sorry for the confusion, but the fact that ExecGetUpdatedCols() returns an
> > empty set is an omission rather than deliberate choice. Assuming we fix that,
> > the result of ExecGetExtraUpdatedCols() does matter. Thus we should copy the
> > related pg_attrdef entries, as I suggest in this patch.
> >
> > Another question is how serious problem it is that ExecGetUpdatedCols()
> > returns empty set. AFAICS, "indexUnchanged" does not affect correctness - it's
> > is only a hint that helps the btree AM decide whent the bottom-up deletion and
> > de-duplication techniques should (not) be used. I'm not sure it's easy to
> > update the set for individual UPDATEs: the UPDATE commands REPACK replays
> > originate from different SQL queries and the logical decoding does not
> > transfer this information.
> >
> > Even then, I think it'd be "less bad" to have ExecGetUpdatedCols() return a
> > set containing all the attributes rather than empty set. That is, avoid using
> > the btree optimizations altogether rather than allow them them when not
> > appropriate. However, per index_unchanged_by_update(), if ExecGetUpdatedCols()
> > tells that all columns are updated, the result of ExecGetExtraUpdatedCols()
> > does not matter.  Nevertheless, I'd still slightly prefer copying the
> > pg_attrdef entries to hacking the executor.
> 
> Agreed, thanks for the correction. Relying on the empty ExecGetUpdatedCols()
> set was the weak point of my version -- it's an omission, not something to
> build a second approximation on. Copying the pg_attrdef entries fixes the
> inconsistency at the root, so let's go with your approach.
> 
> I applied the patch and ran it through an injection-point reproducer
> (cassert). Without the fix the bug reproduces (ERROR: no generation
> expression found for column number 3 ...); with it, REPACK CONCURRENTLY
> succeeds under a concurrent non-HOT UPDATE for a STORED generated column, an
> index directly on the generated column, and a VIRTUAL column, with correct
> values afterwards. Your repack.spec change passes.
> 
> The approach is right and I've confirmed it fixes the bug, so +1 from me in
> this direction.

Thanks for checking. Here, 0002 tries to fix the problem of empty updatedCols,
as I proposed above.

0001 fixes two minor coding issues that I found when writing 0002.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

  11. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Ewan Young <kdbase.hack@gmail.com> — 2026-07-02T01:39:42Z

    On Wed, Jul 1, 2026 at 9:57 PM Antonin Houska <ah@cybertec.at> wrote:
>
> Ewan Young <kdbase.hack@gmail.com> wrote:
>
> > On Mon, Jun 22, 2026 at 7:12 PM Antonin Houska <ah@cybertec.at> wrote:
> > >
> > > Ewan Young <kdbase.hack@gmail.com> wrote:
> > >
> > > > The transient heap built by make_new_heap() is intentionally created
> > > > without the old table's defaults and constraints, so it has no generation
> > > > expressions for its generated columns, even though the tuple descriptor
> > > > still has attgenerated set.
> > > >
> > > > When apply_concurrent_update() replays a non-HOT update, it calls
> > > > ExecInsertIndexTuples() with EIIT_IS_UPDATE. To decide whether to pass
> > > > the "indexUnchanged" hint, that calls index_unchanged_by_update() ->
> > > > ExecGetExtraUpdatedCols() -> ExecInitGenerated(), which looks up the
> > > > generation expression of each generated column via build_column_default()
> > > > and errors out when it finds none on the transient heap.
> > > >
> > > > The apply path does not need to recompute generated columns at all: the
> > > > decoded tuple already carries the correct value, and it is only inserted.
> > > > Note also that ExecGetUpdatedCols() already returns an empty set for this
> > > > ResultRelInfo, because it is not part of any range table -- so the
> > > > indexUnchanged determination here is already approximate.
> > >
> > > I'm sorry for the confusion, but the fact that ExecGetUpdatedCols() returns an
> > > empty set is an omission rather than deliberate choice. Assuming we fix that,
> > > the result of ExecGetExtraUpdatedCols() does matter. Thus we should copy the
> > > related pg_attrdef entries, as I suggest in this patch.
> > >
> > > Another question is how serious problem it is that ExecGetUpdatedCols()
> > > returns empty set. AFAICS, "indexUnchanged" does not affect correctness - it's
> > > is only a hint that helps the btree AM decide whent the bottom-up deletion and
> > > de-duplication techniques should (not) be used. I'm not sure it's easy to
> > > update the set for individual UPDATEs: the UPDATE commands REPACK replays
> > > originate from different SQL queries and the logical decoding does not
> > > transfer this information.
> > >
> > > Even then, I think it'd be "less bad" to have ExecGetUpdatedCols() return a
> > > set containing all the attributes rather than empty set. That is, avoid using
> > > the btree optimizations altogether rather than allow them them when not
> > > appropriate. However, per index_unchanged_by_update(), if ExecGetUpdatedCols()
> > > tells that all columns are updated, the result of ExecGetExtraUpdatedCols()
> > > does not matter.  Nevertheless, I'd still slightly prefer copying the
> > > pg_attrdef entries to hacking the executor.
> >
> > Agreed, thanks for the correction. Relying on the empty ExecGetUpdatedCols()
> > set was the weak point of my version -- it's an omission, not something to
> > build a second approximation on. Copying the pg_attrdef entries fixes the
> > inconsistency at the root, so let's go with your approach.
> >
> > I applied the patch and ran it through an injection-point reproducer
> > (cassert). Without the fix the bug reproduces (ERROR: no generation
> > expression found for column number 3 ...); with it, REPACK CONCURRENTLY
> > succeeds under a concurrent non-HOT UPDATE for a STORED generated column, an
> > index directly on the generated column, and a VIRTUAL column, with correct
> > values afterwards. Your repack.spec change passes.
> >
> > The approach is right and I've confirmed it fixes the bug, so +1 from me in
> > this direction.
>
> Thanks for checking. Here, 0002 tries to fix the problem of empty updatedCols,
> as I proposed above.
>
> 0001 fixes two minor coding issues that I found when writing 0002.

Both look good to me. 0001 is a clear improvement — makeNode() is the right
idiom, and NULL rather than 0 for the pointer argument.

For 0002, I agree with the approach: since indexUnchanged only feeds the btree
bottom-up-deletion / dedup heuristics, treating all columns as updated (so the
hint is always false) is the safe choice — better to forgo those optimizations
than to misuse them. I checked the mechanism against the current code and it
holds: with ri_RangeTableIndex = 1, ExecGetUpdatedCols() returns the full set,
so index_unchanged_by_update() always sees a changed key column.

+1 on both.

>
> --
> Antonin Houska
> Web: https://www.cybertec-postgresql.com
>


-- 
Regards,
Ewan Young

  12. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Álvaro Herrera <alvherre@kurilemu.de> — 2026-07-03T11:12:39Z

    Hello,

On 2026-Jun-22, Ewan Young wrote:

> I applied the patch and ran it through an injection-point reproducer
> (cassert). Without the fix the bug reproduces (ERROR: no generation
> expression found for column number 3 ...); with it, REPACK CONCURRENTLY
> succeeds under a concurrent non-HOT UPDATE for a STORED generated column, an
> index directly on the generated column, and a VIRTUAL column, with correct
> values afterwards. Your repack.spec change passes.
> 
> The approach is right and I've confirmed it fixes the bug, so +1 from me in
> this direction.

Cool, thanks for reviewing -- I have pushed this fix, with some
stylistic changes and one bigger change: these catalog rows are only
needed in concurrent mode, so there was no reason to copy them in the
other case.  So I restricted the copying to that case.

I've been looking at the other proposed change, and I agree with it.
Here's it, again with some style changes, and only one other proposed
change: for setting up updatedCols, ignore dropped columns.  I don't
think this should change anything in practice, but it just feels wrong
to claim that a dropped column is being changed by an update.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

  13. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Ewan Young <kdbase.hack@gmail.com> — 2026-07-03T14:20:42Z

    Thanks, Álvaro.

On Fri, Jul 3, 2026 at 7:12 PM Alvaro Herrera <alvherre@kurilemu.de> wrote:
>
> Hello,
>
> On 2026-Jun-22, Ewan Young wrote:
>
> > I applied the patch and ran it through an injection-point reproducer
> > (cassert). Without the fix the bug reproduces (ERROR: no generation
> > expression found for column number 3 ...); with it, REPACK CONCURRENTLY
> > succeeds under a concurrent non-HOT UPDATE for a STORED generated column, an
> > index directly on the generated column, and a VIRTUAL column, with correct
> > values afterwards. Your repack.spec change passes.
> >
> > The approach is right and I've confirmed it fixes the bug, so +1 from me in
> > this direction.
>
> Cool, thanks for reviewing -- I have pushed this fix, with some
> stylistic changes and one bigger change: these catalog rows are only
> needed in concurrent mode, so there was no reason to copy them in the
> other case.  So I restricted the copying to that case.
>
> I've been looking at the other proposed change, and I agree with it.
> Here's it, again with some style changes, and only one other proposed
> change: for setting up updatedCols, ignore dropped columns.  I don't
> think this should change anything in practice, but it just feels wrong
> to claim that a dropped column is being changed by an update.

I read through the range-table change and it looks correct.

Agreed on skipping dropped columns — a dropped attribute can never be
an index key column, so its presence in updatedCols has no functional effect;
leaving it out is a pure clarity win.

Over-claiming "all columns updated" is the safe direction: it only
affects the btree
indexUnchanged / bottom-up-deletion hint, never correctness.  And for the
concurrent-apply path losing that hint should rarely matter, so erring
this way looks right.

+1 to commit.

>
> --
> Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

-- 
Regards,
Ewan Young

  14. Re: REPACK CONCURRENTLY fails on tables with generated columns

    Antonin Houska <ah@cybertec.at> — 2026-07-03T17:26:17Z

    Alvaro Herrera <alvherre@kurilemu.de> wrote:

> On 2026-Jun-22, Ewan Young wrote:
> 
> > I applied the patch and ran it through an injection-point reproducer
> > (cassert). Without the fix the bug reproduces (ERROR: no generation
> > expression found for column number 3 ...); with it, REPACK CONCURRENTLY
> > succeeds under a concurrent non-HOT UPDATE for a STORED generated column, an
> > index directly on the generated column, and a VIRTUAL column, with correct
> > values afterwards. Your repack.spec change passes.
> > 
> > The approach is right and I've confirmed it fixes the bug, so +1 from me in
> > this direction.
> 
> Cool, thanks for reviewing -- I have pushed this fix, with some
> stylistic changes and one bigger change: these catalog rows are only
> needed in concurrent mode, so there was no reason to copy them in the
> other case.  So I restricted the copying to that case.

Good point, thanks.

> I've been looking at the other proposed change, and I agree with it.
> Here's it, again with some style changes, and only one other proposed
> change: for setting up updatedCols, ignore dropped columns.  I don't
> think this should change anything in practice, but it just feels wrong
> to claim that a dropped column is being changed by an update.

+1

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com