From 35b808572be533eba3565b68b9f96295f66c5cd1 Mon Sep 17 00:00:00 2001 From: Ewan Young Date: Sat, 13 Jun 2026 00:14:38 +0800 Subject: [PATCH v1] Fix REPACK CONCURRENTLY on tables with generated columns When REPACK (CONCURRENTLY) applies concurrent data changes to the transient new heap during the catch-up phase, a non-HOT UPDATE goes through ExecInsertIndexTuples(), which may call ExecGetExtraUpdatedCols() to decide whether to pass the "indexUnchanged" hint to the index AM. That in turn calls ExecInitGenerated(), which looks up the generation expressions of the relation's STORED generated columns. The transient heap is intentionally created without the defaults and constraints of the original table (see make_new_heap()), so it has no generation expressions, and the lookup failed with: ERROR: no generation expression found for column number %d of table "%s" This aborted REPACK (CONCURRENTLY) on any table having a STORED generated column whenever a concurrent UPDATE that required index maintenance was applied during catch-up. The apply path replays already-decoded tuples, which already carry the correct generated values; it must not recompute them. Mark the generated-column info of the apply ResultRelInfo as already computed (with an empty set) so that ExecInsertIndexTuples() does not invoke ExecInitGenerated(). This is consistent with ExecGetUpdatedCols(), which already returns an empty set here because this ResultRelInfo is not part of any range table. Add an isolation test exercising a non-HOT UPDATE applied during REPACK (CONCURRENTLY) catch-up on a table with a generated column. --- src/backend/commands/repack.c | 14 ++++ src/test/modules/injection_points/Makefile | 1 + .../expected/repack_generated.out | 36 ++++++++++ src/test/modules/injection_points/meson.build | 1 + .../specs/repack_generated.spec | 65 +++++++++++++++++++ 5 files changed, 117 insertions(+) create mode 100644 src/test/modules/injection_points/expected/repack_generated.out create mode 100644 src/test/modules/injection_points/specs/repack_generated.spec diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index ec100e3eef5..e9e90114054 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -3011,6 +3011,20 @@ initialize_change_context(ChangeContext *chgcxt, InitResultRelInfo(chgcxt->cc_rri, relation, 0, 0, 0); ExecOpenIndices(chgcxt->cc_rri, false); + /* + * The new heap is created without the defaults and constraints of the old + * heap (see make_new_heap()), so it has no generation expressions for its + * generated columns. When applying concurrent changes we must not try to + * compute those expressions: the decoded tuples already carry the correct + * generated values, and we only insert them. Mark the generated-column + * info as already computed (with an empty set) so that the index + * maintenance performed by ExecInsertIndexTuples() does not call + * ExecInitGenerated(), which would fail to find the missing expressions. + * This matches ExecGetUpdatedCols(), which is already empty here because + * this ResultRelInfo is not in any range table. + */ + chgcxt->cc_rri->ri_extraUpdatedCols_valid = true; + /* * The table's relcache entry already has the relcache entry for the * identity index; find that. diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index c01d2fb095c..0e5d4ba9973 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -15,6 +15,7 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress ISOLATION = basic \ inplace \ repack \ + repack_generated \ repack_temporal \ repack_temporal_multirange \ repack_toast \ diff --git a/src/test/modules/injection_points/expected/repack_generated.out b/src/test/modules/injection_points/expected/repack_generated.out new file mode 100644 index 00000000000..fc7cf2011c5 --- /dev/null +++ b/src/test/modules/injection_points/expected/repack_generated.out @@ -0,0 +1,36 @@ +Parsed test spec with 2 sessions + +starting permutation: wait_before_lock change_existing wakeup_before_lock check +injection_points_attach +----------------------- + +(1 row) + +step wait_before_lock: + REPACK (CONCURRENTLY) repack_test USING INDEX repack_test_pkey; + +step change_existing: + UPDATE repack_test SET v = v + 1 WHERE i = 1; + +step wakeup_before_lock: + SELECT injection_points_wakeup('repack-concurrently-before-lock'); + +injection_points_wakeup +----------------------- + +(1 row) + +step wait_before_lock: <... completed> +step check: + SELECT i, v, g FROM repack_test ORDER BY i; + +i|v| g +-+-+-- +1|2|20 +(1 row) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 59dba1cb023..fb92cfa948a 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -46,6 +46,7 @@ tests += { 'basic', 'inplace', 'repack', + 'repack_generated', 'repack_temporal', 'repack_temporal_multirange', 'repack_toast', diff --git a/src/test/modules/injection_points/specs/repack_generated.spec b/src/test/modules/injection_points/specs/repack_generated.spec new file mode 100644 index 00000000000..3d057f490f7 --- /dev/null +++ b/src/test/modules/injection_points/specs/repack_generated.spec @@ -0,0 +1,65 @@ +# Test REPACK (CONCURRENTLY) on a table with a STORED generated column. +# +# When a concurrent UPDATE that requires index maintenance (a non-HOT update) +# is applied to the transient heap during the catch-up phase, the index +# maintenance code used to try to look up the generated column's expression on +# the transient heap. That heap is created without defaults/constraints, so +# the lookup failed with "no generation expression found ...". The decoded +# tuple already carries the correct generated value, so REPACK must just apply +# it and recompute nothing. +setup +{ + CREATE EXTENSION injection_points; + + CREATE TABLE repack_test(i int PRIMARY KEY, v int, + g int GENERATED ALWAYS AS (v * 10) STORED); + -- Index on v so that an UPDATE of v is a non-HOT update, forcing the + -- concurrent-change apply path through index maintenance. + CREATE INDEX repack_test_v_idx ON repack_test(v); + INSERT INTO repack_test(i, v) VALUES (1, 1); +} + +teardown +{ + DROP TABLE repack_test; + DROP EXTENSION injection_points; +} + +session s1 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('repack-concurrently-before-lock', 'wait'); +} +# Perform the initial load and wait for s2 to do a data change. +step wait_before_lock +{ + REPACK (CONCURRENTLY) repack_test USING INDEX repack_test_pkey; +} +teardown +{ + SELECT injection_points_detach('repack-concurrently-before-lock'); +} + +session s2 +# A non-HOT UPDATE (v is indexed) of the existing row, applied during catch-up. +step change_existing +{ + UPDATE repack_test SET v = v + 1 WHERE i = 1; +} +step wakeup_before_lock +{ + SELECT injection_points_wakeup('repack-concurrently-before-lock'); +} +# REPACK must succeed and the concurrent change must be reflected, with the +# generated column recomputed correctly (g = v * 10). +step check +{ + SELECT i, v, g FROM repack_test ORDER BY i; +} + +permutation + wait_before_lock + change_existing + wakeup_before_lock + check -- 2.47.3