From 6e4d941cabbc16c60d778345558612208d78d881 Mon Sep 17 00:00:00 2001 From: Srinath Reddy Sadipiralla Date: Wed, 25 Mar 2026 20:53:09 +0530 Subject: [PATCH 1/1] Check for transaction block early in ExecRepack Currently, executing REPACK (CONCURRENTLY) without a table name inside a transaction block throws the error "REPACK CONCURRENTLY requires explicit table name" instead of the expected transaction block error. This occurs because ExecRepack() validates the parsed options and missing relation before verifying the transaction state. This behavior is inconsistent with other utility commands like VACUUM ,REINDEX, etc; which invoke PreventInTransactionBlock() at the very start of their execution to properly reject execution inside user transactions before validating targets. Add PreventInTransactionBlock to the top of ExecRepack() to enforce the transaction block restriction early. This prevents the user from fixing a missing table error only to immediately hit a transaction block error, and also ensures consistency with rest of the commands. --- src/backend/commands/cluster.c | 39 +++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 423cea26b0b..38c58f6df6e 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -369,6 +369,28 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel) parser_errposition(pstate, opt->location)); } + if (params.options & CLUOPT_CONCURRENT) + { + /* + * Make sure we're not in a transaction block. + * + * The reason is that repack_setup_logical_decoding() could deadlock + * if there's an XID already assigned. It would be possible to run in + * a transaction block if we had no XID, but this restriction is + * simpler for users to understand and we don't lose anything. + */ + PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)"); + } + else + { + /* + * In order to avoid holding locks for too long, we want to process + * each table in its own transaction. This forces us to disallow + * running inside a user transaction block. + */ + PreventInTransactionBlock(isTopLevel, RepackCommandAsString(stmt->command)); + } + /* Determine the lock mode to use. */ lockmode = RepackLockLevel((params.options & CLUOPT_CONCURRENT) != 0); @@ -413,13 +435,6 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel) errmsg("REPACK CONCURRENTLY requires explicit table name")); } - /* - * In order to avoid holding locks for too long, we want to process each - * table in its own transaction. This forces us to disallow running - * inside a user transaction block. - */ - PreventInTransactionBlock(isTopLevel, RepackCommandAsString(stmt->command)); - /* Also, we need a memory context to hold our list of relations */ repack_context = AllocSetContextCreate(PortalContext, "Repack", @@ -605,16 +620,6 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid, */ if (concurrent) { - /* - * Make sure we're not in a transaction block. - * - * The reason is that repack_setup_logical_decoding() could deadlock - * if there's an XID already assigned. It would be possible to run in - * a transaction block if we had no XID, but this restriction is - * simpler for users to understand and we don't lose anything. - */ - PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)"); - check_repack_concurrently_requirements(OldHeap, &ident_idx); } -- 2.43.0