From: Antonin Houska <ah@cybertec.at>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
cc: Srinath Reddy Sadipiralla <srinath2133@gmail.com>,
    Mihail Nikalayeu <mihailnikalayeu@gmail.com>,
    Matthias van de Meent <boekewurm+postgres@gmail.com>,
    Pg Hackers <pgsql-hackers@lists.postgresql.org>,
    Robert Treat <rob@xzilla.net>
Subject: Re: Adding REPACK [concurrently]
In-reply-to: <202603252005.quy5h4oipoxd@alvherre.pgsql>
References: <202603252005.quy5h4oipoxd@alvherre.pgsql>
Comments: In-reply-to Alvaro Herrera <alvherre@alvh.no-ip.org>
   message dated "Wed, 25 Mar 2026 21:12:48 +0100."
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Date: Thu, 26 Mar 2026 15:06:29 +0100
Message-ID: <43861.1774533989@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/43861.1774533989%40localhost>
Precedence: bulk

--=-=-=
Content-Type: text/plain

Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

> (I omitted the last three patches in the series, and
> squashed my proposed changes into 0003, as announced in my previous
> posting.)

I've updated the comment about on-disk attributes in repack_store_change(),
but when verifying it, I hit an error when more than one UPDATEs (in separate
transactions) were executed during a single run of REPACK.

The problem is that reorderbuffer.c sets up an internal (sub)transaction
before replaying each decoded transaction. Therefore the tuple slot should not
be allocated in TopTransactionContext. I chose TopMemoryContext instead.

BTW, if you want to verify that the updated comment is correct, just add
elog(ERROR) next to it and run repack_toast.spec. The statement

	UPDATE repack_test SET i=3 where i=1;

will then reach it.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com


--=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment;
 filename=nocfbot_fix_repack_store_change.diff

diff --git a/src/backend/replication/pgoutput_repack/pgoutput_repack.c b/src/backend/replication/pgoutput_repack/pgoutput_repack.c
index cc9ce615b18..5fe3115509e 100644
--- a/src/backend/replication/pgoutput_repack/pgoutput_repack.c
+++ b/src/backend/replication/pgoutput_repack/pgoutput_repack.c
@@ -202,7 +202,11 @@ repack_store_change(LogicalDecodingContext *ctx, Relation relation,
 		/* Initialize the slot, if not done already */
 		if (dstate->slot == NULL)
 		{
-			MemoryContextSwitchTo(oldcxt);
+			/*
+			 * We are in the decoding worker, so no worries about polluting
+			 * memory of the backend executing REPACK.
+			 */
+			MemoryContextSwitchTo(TopMemoryContext);
 			dstate->slot = MakeSingleTupleTableSlot(desc, &TTSOpsHeapTuple);
 			MemoryContextSwitchTo(dstate->change_cxt);
 		}
@@ -247,8 +251,8 @@ repack_store_change(LogicalDecodingContext *ctx, Relation relation,
 				 * attributes (those actually should never appear on disk), so
 				 * only TOASTed attribute can be seen here.
 				 *
-				 * FIXME in what circumstances can an ONDISK attr appear? Why
-				 * aren't these written separately?
+				 * We get here if the table has external values but only
+				 * in-line values are being updated now.
 				 */
 				Assert(VARATT_IS_EXTERNAL_ONDISK(varlen));
 			}

--=-=-=--