Hi, 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. > > > I think we should have the check for it. > > One idea would be to have a new table AM callback returning true if > > the table AM supports concurrent repack (i.e., its rmgr provides > > rm_decode callback), but I think it's not the right time to introduce > > a new table AM callback. > > Agreed. Adding a new table AM callback needs consensus and is time > consuming. Perhaps we can try for HEAD based on field reports that > require the concurrent repack feature for non-heap table AMs. > > > So restricting it to heap for v19 seems fine > > to me. > > +1. I will send a patch soon.
Please find the attached patch. I didn't add a test, I don't think we need one. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
From 72917b6cd02313e4ffb31e256914d5ad3db43013 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Thu, 27 Aug 2026 20:16:09 +0000 Subject: [PATCH v1] Restrict concurrent repack to the heap access method. Concurrent repack (28d534e2ae0) replays the data changes made during the table rewrite by decoding them from WAL, which only works if the table's access method supports logical decoding. check_concurrent_repack_requirements() didn't check for that. For an access method that doesn't support it, 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. Recognizing logical-decoding support properly would need a new table AM callback, which is more than this open item warrants, so restrict it to heap for now. Backpatch to v19, where concurrent repack was introduced. 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%40nathan Backpatch-through: 19 --- src/backend/commands/repack.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 477c86b2ba6..05c0662a510 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -877,6 +877,20 @@ 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 concurrent + * repack to silently lose the changes made during the rewrite. Detecting + * such support cleanly would need a new table AM callback, so for now + * just restrict the command to heap. + */ + if (rel->rd_rel->relam != HEAP_TABLE_AM_OID) + 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.47.3
