diff --git a/contrib/test_decoding/expected/repack.out b/contrib/test_decoding/expected/repack.out index 1f99f26c1f8..cf93c1f0d3d 100644 --- a/contrib/test_decoding/expected/repack.out +++ b/contrib/test_decoding/expected/repack.out @@ -29,3 +29,25 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a DROP TABLE ptnowner; DROP ROLE regress_ptnowner; +-- Verify that REPACK (CONCURRENTLY) doesn't lose "attmissingval" columns +CREATE TABLE rpk_missing (id int PRIMARY KEY); +INSERT INTO rpk_missing SELECT generate_series(1, 3); +ALTER TABLE rpk_missing ADD COLUMN a int DEFAULT 42; +SELECT * FROM rpk_missing; + id | a +----+---- + 1 | 42 + 2 | 42 + 3 | 42 +(3 rows) + +REPACK (CONCURRENTLY) rpk_missing; +SELECT * FROM rpk_missing; + id | a +----+---- + 1 | 42 + 2 | 42 + 3 | 42 +(3 rows) + +DROP TABLE rpk_missing; diff --git a/contrib/test_decoding/sql/repack.sql b/contrib/test_decoding/sql/repack.sql index c3bfae61f7f..6164dd4235f 100644 --- a/contrib/test_decoding/sql/repack.sql +++ b/contrib/test_decoding/sql/repack.sql @@ -23,3 +23,12 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; DROP ROLE regress_ptnowner; + +-- Verify that REPACK (CONCURRENTLY) doesn't lose "attmissingval" columns +CREATE TABLE rpk_missing (id int PRIMARY KEY); +INSERT INTO rpk_missing SELECT generate_series(1, 3); +ALTER TABLE rpk_missing ADD COLUMN a int DEFAULT 42; +SELECT * FROM rpk_missing; +REPACK (CONCURRENTLY) rpk_missing; +SELECT * FROM rpk_missing; +DROP TABLE rpk_missing; diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 20d3b46e062..2268cc277bc 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2396,10 +2396,10 @@ heap_insert_for_repack(HeapTuple tuple, Relation OldHeap, Relation NewHeap, /* * Subroutine for reform_and_rewrite_tuple and heap_insert_for_repack. * - * Deform the given tuple, set values of dropped columns to NULL, form a new - * tuple and return it. If no attributes need to be changed in this way, a - * copy of the original tuple is returned. Caller is responsible for freeing - * the returned tuple. + * Deform the given tuple, set values of dropped columns to NULL, and fill in + * any values from attmissingval; then form a new tuple and return it. If no + * attributes need to be changed, a copy of the original tuple is returned. + * Caller is responsible for freeing the returned tuple. * * XXX this coding assumes that both relations have the same tupledesc. */ @@ -2411,13 +2411,33 @@ reform_tuple(HeapTuple tuple, Relation OldHeap, Relation NewHeap, TupleDesc newTupDesc = RelationGetDescr(NewHeap); bool needs_reform = false; - /* Skip work if the tuple doesn't need any attributes changed */ - for (int i = 0; i < newTupDesc->natts; i++) + /* + * A short tuple might require values from attmissing val, so activate the + * coding unconditionally in that case. The value might legitimally be + * NULL otherwise, so this is slightly wasteful, but it probably beats + * having to test each attribute for presence of attmissingval each time. + */ + if (HeapTupleHeaderGetNatts(tuple->t_data) < newTupDesc->natts) + needs_reform = true; + + /* + * If the column has been dropped but a value is still present, we can + * optimize storage now by getting rid of it. + */ + if (!needs_reform) { - if (TupleDescCompactAttr(newTupDesc, i)->attisdropped && - !heap_attisnull(tuple, i + 1, newTupDesc)) - needs_reform = true; + for (int i = 0; i < newTupDesc->natts; i++) + { + if (TupleDescCompactAttr(newTupDesc, i)->attisdropped && + !heap_attisnull(tuple, i + 1, newTupDesc)) + { + needs_reform = true; + break; + } + } } + + /* Skip work if no changes are needed */ if (!needs_reform) return heap_copytuple(tuple); diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index bd142ed481c..5813f1c61a5 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -969,6 +969,42 @@ SELECT count(*) 0 (1 row) +-- Verify that table-rewriting maintenance commands preserve attmissingval +-- columns. +CREATE TABLE t (id int PRIMARY KEY); +INSERT INTO t SELECT generate_series(1, 3); +ALTER TABLE t ADD COLUMN a int DEFAULT 42; +ALTER TABLE t ADD COLUMN b int NOT NULL DEFAULT 7 CHECK (b > 0); +VACUUM FULL t; +SELECT * FROM t ORDER BY id; + id | a | b +----+----+--- + 1 | 42 | 7 + 2 | 42 | 7 + 3 | 42 | 7 +(3 rows) + +ALTER TABLE t ADD COLUMN c text DEFAULT 'hello'; +CLUSTER t USING t_pkey; +SELECT * FROM t ORDER BY id; + id | a | b | c +----+----+---+------- + 1 | 42 | 7 | hello + 2 | 42 | 7 | hello + 3 | 42 | 7 | hello +(3 rows) + +ALTER TABLE t ADD COLUMN d int DEFAULT 99; +REPACK t; +SELECT * FROM t ORDER BY id; + id | a | b | c | d +----+----+---+-------+---- + 1 | 42 | 7 | hello | 99 + 2 | 42 | 7 | hello | 99 + 3 | 42 | 7 | hello | 99 +(3 rows) + +DROP TABLE t; -- cleanup DROP FOREIGN TABLE ft1; DROP SERVER s0; diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql index 8b31d317d73..e5d9a3d2fd1 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -653,6 +653,22 @@ SELECT count(*) WHERE attrelid = 'ft1'::regclass AND (attmissingval IS NOT NULL OR atthasmissing); +-- Verify that table-rewriting maintenance commands preserve attmissingval +-- columns. +CREATE TABLE t (id int PRIMARY KEY); +INSERT INTO t SELECT generate_series(1, 3); +ALTER TABLE t ADD COLUMN a int DEFAULT 42; +ALTER TABLE t ADD COLUMN b int NOT NULL DEFAULT 7 CHECK (b > 0); +VACUUM FULL t; +SELECT * FROM t ORDER BY id; +ALTER TABLE t ADD COLUMN c text DEFAULT 'hello'; +CLUSTER t USING t_pkey; +SELECT * FROM t ORDER BY id; +ALTER TABLE t ADD COLUMN d int DEFAULT 99; +REPACK t; +SELECT * FROM t ORDER BY id; +DROP TABLE t; + -- cleanup DROP FOREIGN TABLE ft1; DROP SERVER s0;