pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[Bug]Vacuum full silently NULL out fast default columns
6+ messages / 3 participants
[nested] [flat]
* [Bug]Vacuum full silently NULL out fast default columns
@ 2026-05-04 09:17 SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-05-04 09:17 UTC (permalink / raw)
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Álvaro Herrera <alvherre@kurilemu.de>; Antonin Houska <ah@cybertec.at>
Hi Hackers,
VACUUM FULL silently turns columns added via ALTER TABLE ... ADD COLUMN ...
DEFAULT <const> into NULL
on all pre-existing rows. The issue exists for other operations like
CLUSTER, REPACK.
Repro:
CREATE TABLE t (id int PRIMARY KEY);
INSERT INTO t SELECT generate_series(1,3);
ALTER TABLE t ADD COLUMN x int DEFAULT 42;
SELECT * FROM t; -- (1,42),(2,42),(3,42)
VACUUM FULL t;
SELECT * FROM t; -- (1,NULL),(2,NULL),(3,NULL)
If the column is NOT NULL, the value becomes the type's zero value
instead of NULL, silently bypassing both NOT NULL and any CHECK
constraint declared on it.
Root Cause: fast path in reform_tuple() in heapam_handler.c returns a copy
of the source tuple when no dropped columns need fixing up. The check
doesn't account for short tuples (HeapTupleHeaderGetNatts(t) <
relnatts) that rely on attmissingval to materialize the default. After
the rewrite, finish_heap_swap() calls RelationClearMissing(), clearing
the only source of those values, and the short tuples then read as
NULL.
Fix: force reform when the source tuple is shorter than the new tuple
descriptor.
Patch attached. Added a regression test in fast_default.sql covering
VACUUM FULL, CLUSTER, and REPACK on a table with fast-default columns
including a NOT NULL CHECK column.
Thanks,
Satya
Attachments:
[application/octet-stream] 0001-VACUUM-FULL-silently-NULL-out-fast-default-columns.patch (4.5K, ../../CAHg+QDeoccU5CudrJpmSKZfKZ1gRMNY=5BxSC=JpHgkonzgcOw@mail.gmail.com/3-0001-VACUUM-FULL-silently-NULL-out-fast-default-columns.patch)
download | inline diff:
From 1a3e25855452137d6d7c6db89a243ff8089e939f Mon Sep 17 00:00:00 2001
From: Satya Narlapuram <satyanarlapuram@gmail.com>
Date: Mon, 4 May 2026 08:52:50 +0000
Subject: [PATCH] VACUUM FULL silently NULL out fast-default columns
Commit 28d534e2 introduced reform_tuple() in heapam_handler.c with a
fast path that returns the source tuple verbatim when no dropped
columns require fixing up. The check ignores tuples that are short
due to attmissingval (the fast-default mechanism).
After the rewrite, finish_heap_swap() calls RelationClearMissing(),
clearing the catalog metadata that was the only source of those
values. Short tuples then read as NULL (or the type's zero value if
NOT NULL is in effect, also bypassing CHECK constraints). Affects
VACUUM FULL, CLUSTER, REPACK, and REPACK (CONCURRENTLY).
Force reform when the source tuple is shorter than the new tuple
descriptor so heap_deform_tuple() materializes the missing values
before heap_form_tuple() rebuilds a full-width tuple.
---
src/backend/access/heap/heapam_handler.c | 12 +++++++-
src/test/regress/expected/fast_default.out | 36 ++++++++++++++++++++++
src/test/regress/sql/fast_default.sql | 16 ++++++++++
3 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 20d3b46e..12b27776 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2411,8 +2411,18 @@ reform_tuple(HeapTuple tuple, Relation OldHeap, Relation NewHeap,
TupleDesc newTupDesc = RelationGetDescr(NewHeap);
bool needs_reform = false;
+ /*
+ * If the tuple is short due to missing-value (fast-default) attributes,
+ * we must materialize them now: finish_heap_swap() will subsequently call
+ * RelationClearMissing() on the new relation, dropping the catalog
+ * metadata that is the only source of those values. Without reforming,
+ * those columns would silently become NULL after the rewrite.
+ */
+ if (HeapTupleHeaderGetNatts(tuple->t_data) < newTupDesc->natts)
+ needs_reform = true;
+
/* Skip work if the tuple doesn't need any attributes changed */
- for (int i = 0; i < newTupDesc->natts; i++)
+ for (int i = 0; !needs_reform && i < newTupDesc->natts; i++)
{
if (TupleDescCompactAttr(newTupDesc, i)->attisdropped &&
!heap_attisnull(tuple, i + 1, newTupDesc))
diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out
index bd142ed4..5813f1c6 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 8b31d317..e5d9a3d2 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;
--
2.43.0
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [Bug]Vacuum full silently NULL out fast default columns
@ 2026-05-04 13:40 Tom Lane <tgl@sss.pgh.pa.us>
parent: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2026-05-04 13:40 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Álvaro Herrera <alvherre@kurilemu.de>; Antonin Houska <ah@cybertec.at>
SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> writes:
> VACUUM FULL silently turns columns added via ALTER TABLE ... ADD COLUMN ...
> DEFAULT <const> into NULL
> on all pre-existing rows. The issue exists for other operations like
> CLUSTER, REPACK.
That is a seriously awful bug. Fortunately it is not in any shipping
release. A quick bisect run agrees that it broke here:
28d534e2ae0ac888b5460f977a10cd9bb017ef98 is the first bad commit
commit 28d534e2ae0ac888b5460f977a10cd9bb017ef98 (HEAD)
Author: Álvaro Herrera <alvherre@kurilemu.de>
Date: Mon Apr 6 21:55:08 2026 +0200
Add CONCURRENTLY option to REPACK
> Patch attached. Added a regression test in fast_default.sql covering
> VACUUM FULL, CLUSTER, and REPACK on a table with fast-default columns
> including a NOT NULL CHECK column.
I don't know if this is the best code fix (I don't like putting extra
checks into a loop condition like this). But I agree we need some
more tests covering this area.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [Bug]Vacuum full silently NULL out fast default columns
@ 2026-05-04 14:26 SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-05-04 14:26 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Álvaro Herrera <alvherre@kurilemu.de>; Antonin Houska <ah@cybertec.at>
Hi,
On Mon, May 4, 2026 at 6:40 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> writes:
> > VACUUM FULL silently turns columns added via ALTER TABLE ... ADD COLUMN
> ...
> > DEFAULT <const> into NULL
> > on all pre-existing rows. The issue exists for other operations like
> > CLUSTER, REPACK.
>
> That is a seriously awful bug. Fortunately it is not in any shipping
> release. A quick bisect run agrees that it broke here:
>
> 28d534e2ae0ac888b5460f977a10cd9bb017ef98 is the first bad commit
> commit 28d534e2ae0ac888b5460f977a10cd9bb017ef98 (HEAD)
> Author: Álvaro Herrera <alvherre@kurilemu.de>
> Date: Mon Apr 6 21:55:08 2026 +0200
>
> Add CONCURRENTLY option to REPACK
>
> > Patch attached. Added a regression test in fast_default.sql covering
> > VACUUM FULL, CLUSTER, and REPACK on a table with fast-default columns
> > including a NOT NULL CHECK column.
>
> I don't know if this is the best code fix (I don't like putting extra
> checks into a loop condition like this). But I agree we need some
> more tests covering this area.
>
Thanks for reviewing! Attached v2 patch. Agreed, tried to optimize LOC in
V1. Before the change
loop was not breaking early, I fixed that as well in V2.
Thanks,
Satya
Attachments:
[application/octet-stream] v2-0001-VACUUM-FULL-silently-NULL-out-fast-default-columns.patch (4.7K, ../../CAHg+QDeteXPbPFsThTGTL=qp4dgiP1AH1kKktifcksDbiaRROw@mail.gmail.com/3-v2-0001-VACUUM-FULL-silently-NULL-out-fast-default-columns.patch)
download | inline diff:
From 0d1b3254184cda790bc880d264295a6b5b93a151 Mon Sep 17 00:00:00 2001
From: Satya Narlapuram <satyanarlapuram@gmail.com>
Date: Mon, 4 May 2026 08:52:50 +0000
Subject: [PATCH] VACUUM FULL silently NULL out fast-default columns
Commit 28d534e2 introduced reform_tuple() in heapam_handler.c with a
fast path that returns the source tuple verbatim when no dropped
columns require fixing up. The check ignores tuples that are short
due to attmissingval (the fast-default mechanism).
After the rewrite, finish_heap_swap() calls RelationClearMissing(),
clearing the catalog metadata that was the only source of those
values. Short tuples then read as NULL (or the type's zero value if
NOT NULL is in effect, also bypassing CHECK constraints). Affects
VACUUM FULL, CLUSTER, REPACK, and REPACK (CONCURRENTLY).
Force reform when the source tuple is shorter than the new tuple
descriptor so heap_deform_tuple() materializes the missing values
before heap_form_tuple() rebuilds a full-width tuple.
---
src/backend/access/heap/heapam_handler.c | 24 ++++++++++++---
src/test/regress/expected/fast_default.out | 36 ++++++++++++++++++++++
src/test/regress/sql/fast_default.sql | 16 ++++++++++
3 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 20d3b46e..d7c07e08 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2411,12 +2411,28 @@ reform_tuple(HeapTuple tuple, Relation OldHeap, Relation NewHeap,
TupleDesc newTupDesc = RelationGetDescr(NewHeap);
bool needs_reform = false;
+ /*
+ * If the tuple is short due to missing-value (fast-default) attributes,
+ * we must materialize them now: finish_heap_swap() will subsequently call
+ * RelationClearMissing() on the new relation, dropping the catalog
+ * metadata that is the only source of those values. Without reforming,
+ * those columns would silently become NULL after the rewrite.
+ */
+ if (HeapTupleHeaderGetNatts(tuple->t_data) < newTupDesc->natts)
+ needs_reform = true;
+
/* Skip work if the tuple doesn't need any attributes changed */
- for (int i = 0; i < newTupDesc->natts; i++)
+ 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;
+ }
+ }
}
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 bd142ed4..5813f1c6 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 8b31d317..e5d9a3d2 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;
--
2.43.0
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [Bug]Vacuum full silently NULL out fast default columns
@ 2026-05-04 17:41 Álvaro Herrera <alvherre@kurilemu.de>
parent: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Álvaro Herrera @ 2026-05-04 17:41 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>
On 2026-May-04, SATYANARAYANA NARLAPURAM wrote:
> On Mon, May 4, 2026 at 6:40 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > A quick bisect run agrees that it broke here:
> >
> > 28d534e2ae0ac888b5460f977a10cd9bb017ef98 is the first bad commit
> > commit 28d534e2ae0ac888b5460f977a10cd9bb017ef98 (HEAD)
> > Author: Álvaro Herrera <alvherre@kurilemu.de>
> > Date: Mon Apr 6 21:55:08 2026 +0200
> >
> > Add CONCURRENTLY option to REPACK
Right.
> Thanks for reviewing! Attached v2 patch. Agreed, tried to optimize LOC in
> V1. Before the change loop was not breaking early, I fixed that as
> well in V2.
Yeah, this seems a good approach to me. I propose some more comment
updates though, and I also thought it'd be a good idea to add a test for
REPACK CONCURRENTLY while at it.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Attachments:
[text/x-diff] v3-0001-Don-t-lose-column-values-on-REPACK.patch (7.3K, ../../afjYnSJeXoa2LFaU@alvherre.pgsql/2-v3-0001-Don-t-lose-column-values-on-REPACK.patch)
download | inline diff:
From f562935d3c4a2664dd9a6a7aa2741b263c57ccfb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Mon, 4 May 2026 17:19:50 +0200
Subject: [PATCH v3] Don't lose column values on REPACK
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 28d534e2ae0a introduced reform_tuple() with a fast path that
returns the source tuple verbatim when no dropped columns require fixing
up. I (Álvaro) failed to realize that this broke handling of columns
with a 'missingval' defined: after a VACUUM FULL, CLUSTER, or REPACK
operation, the catalogued missingval is thrown away, so the tuples are
no longer correct.
Fix by forcing the rewrite when the tuple is shorter than the tuple
descriptor.
Author: Satya Narlapuram <satyanarlapuram@gmail.com>
Discussion: https://postgr.es/m/CAHg+QDeoccU5CudrJpmSKZfKZ1gRMNY=5BxSC=JpHgkonzgcOw@mail.gmail.com
---
contrib/test_decoding/expected/repack.out | 22 +++++++++++++
contrib/test_decoding/sql/repack.sql | 9 +++++
src/backend/access/heap/heapam_handler.c | 38 +++++++++++++++++-----
src/test/regress/expected/fast_default.out | 36 ++++++++++++++++++++
src/test/regress/sql/fast_default.sql | 16 +++++++++
5 files changed, 112 insertions(+), 9 deletions(-)
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;
--
2.47.3
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [Bug]Vacuum full silently NULL out fast default columns
@ 2026-05-04 18:08 SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
parent: Álvaro Herrera <alvherre@kurilemu.de>
0 siblings, 1 reply; 6+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-05-04 18:08 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>
Hi,
On Mon, May 4, 2026 at 10:41 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> On 2026-May-04, SATYANARAYANA NARLAPURAM wrote:
>
> > On Mon, May 4, 2026 at 6:40 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> > > A quick bisect run agrees that it broke here:
> > >
> > > 28d534e2ae0ac888b5460f977a10cd9bb017ef98 is the first bad commit
> > > commit 28d534e2ae0ac888b5460f977a10cd9bb017ef98 (HEAD)
> > > Author: Álvaro Herrera <alvherre@kurilemu.de>
> > > Date: Mon Apr 6 21:55:08 2026 +0200
> > >
> > > Add CONCURRENTLY option to REPACK
>
> Right.
>
> > Thanks for reviewing! Attached v2 patch. Agreed, tried to optimize LOC in
> > V1. Before the change loop was not breaking early, I fixed that as
> > well in V2.
>
> Yeah, this seems a good approach to me. I propose some more comment
> updates though, and I also thought it'd be a good idea to add a test for
> REPACK CONCURRENTLY while at it.
>
This patch LGTM. It applied cleanly and all the tests passed.
Thanks,
Satya
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [Bug]Vacuum full silently NULL out fast default columns
@ 2026-05-05 08:27 Álvaro Herrera <alvherre@kurilemu.de>
parent: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
0 siblings, 0 replies; 6+ messages in thread
From: Álvaro Herrera @ 2026-05-05 08:27 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>
On 2026-May-04, SATYANARAYANA NARLAPURAM wrote:
> This patch LGTM. It applied cleanly and all the tests passed.
OK, thanks, pushed.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"The problem with the future is that it keeps turning into the present"
(Hobbes)
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-05-05 08:27 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-04 09:17 [Bug]Vacuum full silently NULL out fast default columns SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
2026-05-04 13:40 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-05-04 14:26 ` SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
2026-05-04 17:41 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-05-04 18:08 ` SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
2026-05-05 08:27 ` Álvaro Herrera <alvherre@kurilemu.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox