public inbox for [email protected]
help / color / mirror / Atom feedFrom: Srinath Reddy Sadipiralla <[email protected]>
To: Antonin Houska <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
Date: Wed, 3 Jun 2026 15:55:18 +0530
Message-ID: <CAFC+b6qY8Jr1OkRqRX0KOy6-yauGwKTrw-OhQooYgzpyCwhgTA@mail.gmail.com> (raw)
In-Reply-To: <33766.1780471821@localhost>
References: <[email protected]>
<[email protected]>
<CAFC+b6rpgFUUiU5z_YbF3GJStNqO2Pf7eW96txw=JtEe=_WPzw@mail.gmail.com>
<33766.1780471821@localhost>
On Wed, Jun 3, 2026 at 1:00 PM Antonin Houska <[email protected]> wrote:
> Srinath Reddy Sadipiralla <[email protected]> wrote:
>
> > Could we reject the pgrepack plugin at slot creation instead, in
> > pg_create_logical_replication_slot() and the CREATE_REPLICATION_SLOT
> > command, so misuse gets a clear "reserved for REPACK (CONCURRENTLY)"
> > error up front, before any decoding? REPACK creates its slot directly via
> > ReplicationSlotCreate(), so it's unaffected, and the begin-callback check
> > with magic guard can stay as the internal safety net.
> > Happy to be told this isn't worth special-casing :)
>
> Another possible approach: restrict the use of the plugin to the REPACK
> decoding worker.
>
cool ... that's cleaner, incorporated these changes and added test
,errcode.
--
Thanks :)
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
Attachments:
[application/octet-stream] v2-0002-Reject-the-pgrepack-output-plugin-outside-REPACK-CON.patch (4.7K, 3-v2-0002-Reject-the-pgrepack-output-plugin-outside-REPACK-CON.patch)
download | inline diff:
From 9fff3de85d0c7c67f29fce28f9d366db5a3cf46a Mon Sep 17 00:00:00 2001
From: Srinath Reddy Sadipiralla <[email protected]>
Date: Wed, 3 Jun 2026 15:45:47 +0530
Subject: [PATCH 1/1] Reject the pgrepack output plugin outside REPACK
(CONCURRENTLY)
The pgrepack output plugin is an internal component of REPACK
(CONCURRENTLY) and is not meant to be driven through the generic logical
decoding interface.
---
contrib/test_decoding/expected/repack.out | 4 ++++
contrib/test_decoding/sql/repack.sql | 4 ++++
src/backend/commands/repack_worker.c | 4 +---
src/backend/replication/logical/logical.c | 12 ++++++++++++
src/include/commands/repack.h | 2 ++
5 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/contrib/test_decoding/expected/repack.out b/contrib/test_decoding/expected/repack.out
index 6204e620b43..c9a7bcc7ddc 100644
--- a/contrib/test_decoding/expected/repack.out
+++ b/contrib/test_decoding/expected/repack.out
@@ -99,5 +99,9 @@ REPACK (CONCURRENTLY) repack_conc_replident;
ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_replident"
DETAIL: REPACK (CONCURRENTLY) does not support deferrable primary keys.
HINT: Use ALTER TABLE ... REPLICA IDENTITY USING INDEX to designate another index as replica identity.
+-- The "pgrepack" output plugin is internal to REPACK (CONCURRENTLY); it cannot
+-- be selected through the generic logical decoding interface (bug #19500).
+SELECT pg_create_logical_replication_slot('repack_misuse_slot', 'pgrepack');
+ERROR: The "pgrepack" decoder plugin may only be called by REPACK (CONCURRENTLY).
-- clean up
DROP TABLE repack_conc_replident, clstrpart;
diff --git a/contrib/test_decoding/sql/repack.sql b/contrib/test_decoding/sql/repack.sql
index cea3bd33689..a7cb615e55d 100644
--- a/contrib/test_decoding/sql/repack.sql
+++ b/contrib/test_decoding/sql/repack.sql
@@ -73,5 +73,9 @@ REPACK (CONCURRENTLY) repack_conc_replident;
ALTER TABLE repack_conc_replident ADD PRIMARY KEY (i) DEFERRABLE;
REPACK (CONCURRENTLY) repack_conc_replident;
+-- The "pgrepack" output plugin is internal to REPACK (CONCURRENTLY); it cannot
+-- be selected through the generic logical decoding interface (bug #19500).
+SELECT pg_create_logical_replication_slot('repack_misuse_slot', 'pgrepack');
+
-- clean up
DROP TABLE repack_conc_replident, clstrpart;
diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index f3d2720b246..9ea363f5ea4 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -28,8 +28,6 @@
#include "tcop/tcopprot.h"
#include "utils/memutils.h"
-#define REPL_PLUGIN_NAME "pgrepack"
-
static void RepackWorkerShutdown(int code, Datum arg);
static LogicalDecodingContext *repack_setup_logical_decoding(Oid relid);
static void repack_cleanup_logical_decoding(LogicalDecodingContext *ctx);
@@ -246,7 +244,7 @@ repack_setup_logical_decoding(Oid relid)
* Neither prepare_write nor do_write callback nor update_progress is
* useful for us.
*/
- ctx = CreateInitDecodingContext(REPL_PLUGIN_NAME,
+ ctx = CreateInitDecodingContext(REPACK_PLUGIN_NAME,
NIL,
true,
true,
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 3541fc793e4..28b9c4ea307 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -31,6 +31,7 @@
#include "access/xact.h"
#include "access/xlog_internal.h"
#include "access/xlogutils.h"
+#include "commands/repack.h"
#include "fmgr.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -351,6 +352,17 @@ CreateInitDecodingContext(const char *plugin,
if (plugin == NULL)
elog(ERROR, "cannot initialize logical decoding without a specified plugin");
+ /*
+ * Plugin for REPACK (CONCURRENTLY) is not designed for other uses, such
+ * as the SQL interface. Use the fact that REPACK uses background worker
+ * for the decoding.
+ */
+ if (strcmp(plugin, REPACK_PLUGIN_NAME) == 0 && !AmRepackWorker())
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("The \"%s\" decoder plugin may only be called by %s.",
+ REPACK_PLUGIN_NAME, "REPACK (CONCURRENTLY)")));
+
/* Make sure the passed slot is suitable. These are user facing errors. */
if (SlotIsPhysical(slot))
ereport(ERROR,
diff --git a/src/include/commands/repack.h b/src/include/commands/repack.h
index 45e5440a311..1d5e9dbbe01 100644
--- a/src/include/commands/repack.h
+++ b/src/include/commands/repack.h
@@ -35,6 +35,8 @@ typedef struct ClusterParams
uint32 options; /* bitmask of CLUOPT_* */
} ClusterParams;
+#define REPACK_PLUGIN_NAME "pgrepack"
+
extern PGDLLIMPORT volatile sig_atomic_t RepackMessagePending;
--
2.43.0
view thread (14+ messages) latest in thread
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]
Subject: Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
In-Reply-To: <CAFC+b6qY8Jr1OkRqRX0KOy6-yauGwKTrw-OhQooYgzpyCwhgTA@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