Feedback-ID: ie3de48e3:Fastmail
Date: Wed, 30 Sep 2026 13:48:05 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Radim Marek <radim@boringsql.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	"ah@cybertec.at" <ah@cybertec.at>
Subject: Re: REPACK (CONCURRENTLY) might keep dropped-column data
Message-ID: <arz09-sHiSl3qBN0@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="7velbpnu6zwkp5df"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAJgoLk+aoiyMA5batXhKn9pWg_TQHB9er6U11jN1Wa63CtAgPg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/arz09-sHiSl3qBN0%40alvherre.pgsql>
Precedence: bulk


--7velbpnu6zwkp5df
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

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)

--7velbpnu6zwkp5df
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename=0001-Clear-out-values-from-dropped-columns.patch

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


--7velbpnu6zwkp5df
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment; filename=0002-Crude-test-case.patch

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


--7velbpnu6zwkp5df--