public inbox for [email protected]
help / color / mirror / Atom feedFrom: Srinath Reddy Sadipiralla <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Antonin Houska <[email protected]>
Cc: Mihail Nikalayeu <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Robert Treat <[email protected]>
Subject: Re: Adding REPACK [concurrently]
Date: Fri, 3 Apr 2026 21:28:16 +0530
Message-ID: <CAFC+b6q87-q0XETwMjBRMQk2Yvz8HtY6qPK6kG01HVLGtzjEZA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAFC+b6qB=R7_jwZ5aSFJ=9CaTDbYhkK2hs1f0VS0gxiQC-Pvxw@mail.gmail.com>
<[email protected]>
On Thu, Apr 2, 2026 at 1:57 PM Alvaro Herrera <[email protected]>
wrote:
>
> Yeah, it doesn't apply anymore. I rebased it some time ago, but it
> still failed a few tests -- I'm guessing you don't have either TAP tests
> or injection points enabled, which would explain why you don't see those
> failures. So please rebase it against v49, but also look into making it
> pass all tests (I suggest making it go through CI).
>
Rebased on V50 and check-world passed.
--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
Attachments:
[application/octet-stream] nocfbot-0001-Check-for-transaction-block-early-in-ExecRepack.patch (2.9K, 3-nocfbot-0001-Check-for-transaction-block-early-in-ExecRepack.patch)
download | inline diff:
From 97ef5eca6ebf9d3932d624ed4e63d8387cc89c2a Mon Sep 17 00:00:00 2001
From: Srinath Reddy Sadipiralla <[email protected]>
Date: Fri, 3 Apr 2026 20:53:56 +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/repack.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 2fc30008f59..d932d526235 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -292,6 +292,18 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
LWLockRelease(ProcArrayLock);
+
+ /*
+ * Make sure we're not in a transaction block.
+ *
+ * The reason is that repack_setup_logical_decoding() could wait
+ * indefinitely for our XID to complete. (The deadlock detector would
+ * not recognize it because we'd be waiting for ourselves, i.e. no
+ * real lock conflict.) 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)");
}
/*
@@ -523,21 +535,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
* replica index OID.
*/
if (concurrent)
- {
- /*
- * Make sure we're not in a transaction block.
- *
- * The reason is that repack_setup_logical_decoding() could wait
- * indefinitely for our XID to complete. (The deadlock detector would
- * not recognize it because we'd be waiting for ourselves, i.e. no
- * real lock conflict.) 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);
- }
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
--
2.43.0
view thread (6+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Adding REPACK [concurrently]
In-Reply-To: <CAFC+b6q87-q0XETwMjBRMQk2Yvz8HtY6qPK6kG01HVLGtzjEZA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox