pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedFrom: Álvaro Herrera <alvherre@kurilemu.de>
To: Radim Marek <radim@boringsql.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: ah@cybertec.at <ah@cybertec.at>
Subject: Re: REPACK (CONCURRENTLY) might keep dropped-column data
Date: Wed, 30 Sep 2026 13:48:05 +0200
Message-ID: <arz09-sHiSl3qBN0@alvherre.pgsql> (raw)
In-Reply-To: <CAJgoLk+aoiyMA5batXhKn9pWg_TQHB9er6U11jN1Wa63CtAgPg@mail.gmail.com>
Hello Radim, thanks for testing!
On 2026-Sep-30, Radim Marek wrote:
> Aha, so on my way to office I started thinking and got more silly ideas,
> and now can confirm this is more widespread than logical subscriber use
> case.
Oh, thanks for the simplified test case. We can fix this easily by
setting the column to null in the tuple to write out, as in the attached
patch. The adjust_toast_pointers() function should perhaps be renamed,
and the comment rewritten, since it's no longer just about toast ...
I didn't do that though.
I put together a crude test case to verify with isolationtester.
Without the fix, this reproduces the bloat you saw; with the fix, the
toast table's size after the repack is zero. This needs some more
boiling before being committable, but it suffices to show the problem.
Regards
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"The Gord often wonders why people threaten never to come back after they've
been told never to return" (www.actsofgord.com)
Attachments:
[text/x-diff] 0001-Clear-out-values-from-dropped-columns.patch (1.3K, ../arz09-sHiSl3qBN0@alvherre.pgsql/2-0001-Clear-out-values-from-dropped-columns.patch)
download | inline diff:
From 4ff5f220652e8a2bdb20ce3c6b3958ac7f6e312d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Wed, 30 Sep 2026 13:37:34 +0200
Subject: [PATCH 1/2] Clear out values from dropped columns
Reported-by: Radim Marek <radim@boringsql.com>
Discussion: https://postgr.es/m/CAJgoLkK2UBzB1J9buCsSUbjf7bOqz-o_0=CeiTruU789BhBw-Q@mail.gmail.com
---
src/backend/commands/repack.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 596c1abaf78..eadaec72d30 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -3017,6 +3017,9 @@ restore_tuple(BufFile *file, Relation relation, TupleTableSlot *slot)
/*
* Adjust 'dest' replacing any EXTERNAL_ONDISK toast pointers with the
* corresponding ones from 'src'.
+ *
+ * We also take the opportunity to clear out the values in columns that were
+ * dropped.
*/
static void
adjust_toast_pointers(Relation relation, TupleTableSlot *dest, TupleTableSlot *src)
@@ -3029,7 +3032,10 @@ adjust_toast_pointers(Relation relation, TupleTableSlot *dest, TupleTableSlot *s
varlena *varlena_dst;
if (attr->attisdropped)
+ {
+ dest->tts_isnull[i] = true;
continue;
+ }
if (attr->attlen != -1)
continue;
if (slot_attisnull(dest, i + 1))
--
2.47.3
[text/x-diff] 0002-Crude-test-case.patch (1.9K, ../arz09-sHiSl3qBN0@alvherre.pgsql/3-0002-Crude-test-case.patch)
download | inline diff:
From d4a40603003fadd01456ad74f17c97a219c365f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Wed, 30 Sep 2026 13:43:45 +0200
Subject: [PATCH 2/2] Crude test case
XXX not for commit just yet
---
.../specs/repack_dropped.spec | 52 +++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 src/test/modules/injection_points/specs/repack_dropped.spec
diff --git a/src/test/modules/injection_points/specs/repack_dropped.spec b/src/test/modules/injection_points/specs/repack_dropped.spec
new file mode 100644
index 00000000000..1a1ea085299
--- /dev/null
+++ b/src/test/modules/injection_points/specs/repack_dropped.spec
@@ -0,0 +1,52 @@
+setup {
+ CREATE EXTENSION IF NOT EXISTS injection_points;
+
+ create table repack_dropped (id int primary key, a text, b text);
+ alter table repack_dropped alter column b set storage external;
+ insert into repack_dropped select g, cash_words(g::money), repeat(cash_words(g::money), 10 * g) from generate_series(10, 100) g;
+ create function repack_dropped_f() returns trigger language plpgsql as $$ begin return OLD; end $$;
+ create trigger repack_dropped_t before update on repack_dropped for each row execute function repack_dropped_f();
+ alter table repack_dropped drop column b;
+}
+
+teardown {
+ drop table repack_dropped;
+ drop function repack_dropped_f;
+}
+
+session s1
+
+step s1_size
+{
+ select pg_relation_size(oid), pg_relation_size(reltoastrelid) from pg_class where relname = 'repack_dropped';
+}
+
+step s1_unlock
+{
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+}
+
+session s2
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
+}
+
+step s2_repack
+{
+ repack (concurrently) repack_dropped;
+}
+
+session s3
+step s3_updates
+{
+ update repack_dropped set a = a || a;
+}
+
+permutation
+ s1_size
+ s2_repack
+ s3_updates
+ s1_unlock
+ s1_size
--
2.47.3
view thread (17+ messages) latest in thread
Message-ID: <arz09-sHiSl3qBN0@alvherre.pgsql>
Permalink: ../arz09-sHiSl3qBN0@alvherre.pgsql/
Also on: postgresql.org/message-id/arz09-sHiSl3qBN0@alvherre.pgsql
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@kurilemu.de, radim@boringsql.com, pgsql-hackers@lists.postgresql.org, ah@cybertec.at
Subject: Re: REPACK (CONCURRENTLY) might keep dropped-column data
In-Reply-To: <arz09-sHiSl3qBN0@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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