On Mon, Aug 31, 2026 at 8:46 PM Bharath Rupireddy <[email protected]> wrote: > > Hi, > > On Thu, Aug 27, 2026 at 3:51 PM Bharath Rupireddy > <[email protected]> wrote: > > > > > On Thu, Aug 27, 2026 at 12:57 PM Bharath Rupireddy > > > <[email protected]> wrote: > > > > > > > > > If the table AM doesn't support logical decoding, concurrent repack > > > > > would silently lose some table data as it misses the changes > > > > > happened during the rewrites. > > > > > > > > Yes, that's correct. I came to the same conclusion. > > Please find the attached v3 patch with a test added similar to other > restricted error cases. Sorry for the noise.
I've looked at the patch and have one comment: + if (rel->rd_rel->relam != HEAP_TABLE_AM_OID) Probably it's better to use GetHeapamTableAmRoutine() instead of using heap AM oid. It would allow the following use case the patch adds in the regression tests: +CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler; +CREATE TABLE repack_conc_heap2 (i int) USING heap2; +REPACK (CONCURRENTLY) repack_conc_heap2; REPACK'ing on such a table technically works fine. It's unlikely to drop the check so I think we can add the check without regression tests. I attached the updated patch. Please review it. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
From 9b94ac46635753e0dd26787416865bb00aa397a4 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Tue, 1 Sep 2026 03:29:27 +0000 Subject: [PATCH v4] Restrict REPACK (CONCURRENTLY) to the heap access method. REPACK (CONCURRENTLY) didn't check the table AM of the table being repacked, so if the table AM doesn't support logical decoding, the concurrent changes are never decoded and are silently lost from the rewritten table. Fix by erroring out for tables that use a non-heap access method. Reported-by: Nathan Bossart <[email protected]> Author: Bharath Rupireddy <[email protected]> Reviewed-by: Nathan Bossart <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Discussion: https://postgr.es/m/apBkixO4xAGFoiT3@nathan Backpatch-through: 19 --- doc/src/sgml/ref/repack.sgml | 6 ++++++ src/backend/commands/repack.c | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/doc/src/sgml/ref/repack.sgml b/doc/src/sgml/ref/repack.sgml index 0cb72b6b289..0496cd24af1 100644 --- a/doc/src/sgml/ref/repack.sgml +++ b/doc/src/sgml/ref/repack.sgml @@ -285,6 +285,12 @@ REPACK [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] USING </para> </listitem> + <listitem> + <para> + The table uses an access method other than <literal>heap</literal>. + </para> + </listitem> + <listitem> <para> <command>REPACK</command> is executed inside a transaction block. diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 477c86b2ba6..91e46532132 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -877,6 +877,21 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p) errdetail("%s requires \"wal_level\" to be set to \"replica\" or higher.", "REPACK (CONCURRENTLY)")); + /* + * A table AM that doesn't support logical decoding would cause REPACK + * (CONCURRENTLY) to silently lose the changes made during the rewrite. + * Nothing in TableAmRoutine tells us whether it does, so for now restrict + * to heap. Check the routine rather than the AM OID, so that an AM + * reusing the heap handler still works. + */ + if (rel->rd_tableam != GetHeapamTableAmRoutine()) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot execute %s on relation \"%s\"", + "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)), + errhint("%s is only supported for the \"heap\" access method.", + "REPACK (CONCURRENTLY)")); + /* Data changes in system relations are not logically decoded. */ if (IsCatalogRelation(rel)) ereport(ERROR, -- 2.55.0
