public inbox for [email protected]
help / color / mirror / Atom feedBUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
14+ messages / 7 participants
[nested] [flat]
* BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
@ 2026-05-28 14:54 PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: PG Bug reporting form @ 2026-05-28 14:54 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]
The following bug has been logged on the website:
Bug reference: 19500
Logged by: Nikita Kalinin
Email address: [email protected]
PostgreSQL version: 18.4
Operating system: Fedora 44
Description:
Hi,
It appears that the pgrepack output plugin is accessible through the SQL
logical decoding API, even though the plugin code explicitly indicates that
this interface is not supported. Reading changes from such a slot can cause
a backend process crash in builds with asserts enabled.
The crash is reproducible on the current master branch. Since the web form
does not allow selecting master, I selected the latest available released
version instead.
Steps to reproduce:
CREATE TABLE rp(a int);
SELECT *
FROM pg_create_logical_replication_slot('s_repack', 'pgrepack');
INSERT INTO rp VALUES (1);
SELECT *
FROM pg_logical_slot_get_binary_changes('s_repack', NULL, NULL);
Server log:
2026-05-28 21:32:23.185 +07 [142878] STATEMENT: SELECT *
FROM pg_create_logical_replication_slot('s_repack', 'pgrepack');
TRAP: failed Assert("RelationGetRelid(relation) == private->relid"), File:
"pgrepack.c", Line: 100, PID: 142878
postgres: nkpit postgres [local] SELECT(ExceptionalCondition+0x57)
[0xa2d437]
/tmp/pg/lib/postgresql/pgrepack.so(+0xa99) [0x7f7dd9332a99]
Backtrace:
#0 __pthread_kill_implementation (threadid=<optimized out>,
signo=signo@entry=6,
no_tid=no_tid@entry=0) at pthread_kill.c:44
#1 0x00007f7dd807a8d3 in __pthread_kill_internal (threadid=<optimized out>,
signo=6)
at pthread_kill.c:89
#2 0x00007f7dd801f48e in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
#3 0x00007f7dd80067b3 in __GI_abort () at abort.c:77
#4 0x0000000000a2d458 in ExceptionalCondition (
conditionName=conditionName@entry=0x7f7dd93337c8
"RelationGetRelid(relation) == private->relid",
fileName=fileName@entry=0x7f7dd93337f5 "pgrepack.c",
lineNumber=lineNumber@entry=100) at assert.c:65
#5 0x00007f7dd9332a99 in repack_process_change (ctx=<optimized out>,
txn=<optimized out>, relation=<optimized out>, change=<optimized out>)
at pgrepack.c:100
#6 0x0000000000821223 in change_cb_wrapper (cache=<optimized out>,
txn=<optimized out>, relation=<optimized out>, change=<optimized out>)
at logical.c:1111
#7 0x000000000082d91b in ReorderBufferApplyChange (rb=<optimized out>,
txn=<optimized out>, relation=0x7f7dd848f7e8, change=0x29f71f10,
streaming=false)
at reorderbuffer.c:2080
#8 ReorderBufferProcessTXN (rb=0x29f55f20, txn=0x29f49e30,
commit_lsn=25673024,
snapshot_now=<optimized out>, command_id=command_id@entry=0,
streaming=streaming@entry=false) at reorderbuffer.c:2387
#9 0x000000000082dca9 in ReorderBufferReplay (txn=<optimized out>,
rb=<optimized out>, commit_lsn=<optimized out>, end_lsn=<optimized out>,
commit_time=<optimized out>, origin_id=<optimized out>, origin_lsn=0,
xid=<optimized out>) at reorderbuffer.c:2872
#10 0x000000000082ea38 in ReorderBufferCommit (rb=<optimized out>,
xid=<optimized out>, commit_lsn=<optimized out>, end_lsn=<optimized
out>,
commit_time=<optimized out>, origin_id=<optimized out>,
origin_lsn=<optimized out>)
at reorderbuffer.c:2896
#11 0x000000000081d075 in DecodeCommit (ctx=0x29f3de70, buf=0x7ffe263bd7e0,
parsed=0x7ffe263bd630, xid=695, two_phase=false) at decode.c:755
#12 xact_decode (ctx=0x29f3de70, buf=0x7ffe263bd7e0) at decode.c:254
#13 0x000000000081cbaa in LogicalDecodingProcessRecord
(ctx=ctx@entry=0x29f3de70,
record=<optimized out>) at decode.c:117
#14 0x0000000000823b71 in pg_logical_slot_get_changes_guts
(fcinfo=0x29f2d400,
confirm=confirm@entry=true, binary=binary@entry=true) at
logicalfuncs.c:267
#15 0x0000000000823d13 in pg_logical_slot_get_binary_changes
(fcinfo=<optimized out>)
at logicalfuncs.c:354
#16 0x00000000006b7cb5 in ExecMakeTableFunctionResult (setexpr=0x29f279c8,
econtext=0x29f27818, argContext=<optimized out>,
expectedDesc=0x29f2f348,
randomAccess=false) at execSRF.c:235
#17 0x00000000006ccad7 in FunctionNext (node=0x29f27608) at
nodeFunctionscan.c:95
#18 0x00000000006ac21a in ExecProcNode (node=0x29f27608)
at ../../../src/include/executor/executor.h:327
#19 ExecutePlan (queryDesc=0x29e40780, operation=CMD_SELECT,
sendTuples=true,
numberTuples=0, direction=<optimized out>, dest=0x29f29618) at
execMain.c:1736
#20 standard_ExecutorRun (queryDesc=0x29e40780, direction=<optimized out>,
count=0)
at execMain.c:377
#21 0x00000000008c5f98 in PortalRunSelect (portal=portal@entry=0x29eb7130,
forward=forward@entry=true, count=0, count@entry=9223372036854775807,
dest=dest@entry=0x29f29618) at pquery.c:917
#22 0x00000000008c767e in PortalRun (portal=portal@entry=0x29eb7130,
count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=true,
dest=dest@entry=0x29f29618, altdest=altdest@entry=0x29f29618,
qc=qc@entry=0x7ffe263bdd50) at pquery.c:761
#23 0x00000000008c3308 in exec_simple_query (
query_string=0x29e13800 "SELECT *\n FROM
pg_logical_slot_get_binary_changes('s_repack', NULL, NULL);") at
postgres.c:1290
#24 0x00000000008c4de1 in PostgresMain (dbname=<optimized out>,
username=<optimized out>) at postgres.c:4856
#25 0x00000000008beddd in BackendMain (startup_data=<optimized out>,
startup_data_len=<optimized out>) at backend_startup.c:124
#26 0x00000000007fed6e in postmaster_child_launch (child_type=<optimized
out>,
child_slot=1, startup_data=startup_data@entry=0x7ffe263be1a0,
startup_data_len=startup_data_len@entry=24,
client_sock=client_sock@entry=0x7ffe263be1c0) at launch_backend.c:268
#27 0x0000000000802776 in BackendStartup (client_sock=0x7ffe263be1c0)
at postmaster.c:3627
#28 ServerLoop () at postmaster.c:1728
#29 0x0000000000804239 in PostmasterMain (argc=argc@entry=3,
argv=argv@entry=0x29dbcfe0) at postmaster.c:1415
#30 0x00000000004a1b48 in main (argc=3, argv=0x29dbcfe0) at main.c:231
postgres=# select version();
version
-------------------------------------------------------------------------------------------------------------
PostgreSQL 19devel on x86_64-pc-linux-gnu, compiled by gcc (GCC) 16.1.1
20260515 (Red Hat 16.1.1-2), 64-bit
(1 row)
Is this considered normal behavior for the pgrepack plugin, i.e. essentially
a “don’t do that” situation?
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
@ 2026-05-29 14:23 ` Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Álvaro Herrera @ 2026-05-29 14:23 UTC (permalink / raw)
To: [email protected]; [email protected]
Hi,
On 2026-05-28, PG Bug reporting form wrote:
> It appears that the pgrepack output plugin is accessible through the SQL
> logical decoding API, even though the plugin code explicitly indicates that
> this interface is not supported. Reading changes from such a slot can cause
> a backend process crash in builds with asserts enabled.
> Is this considered normal behavior for the pgrepack plugin, i.e. essentially
> a “don’t do that” situation?
Yeah, I would like to have a way to prevent this, if only for user-friendliness, but it's not terribly pressing since only a role with REPLICATION privs can create the replication slot, which as I recall are already pretty powerful.
--
Álvaro Herrera
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
@ 2026-06-01 17:12 ` Álvaro Herrera <[email protected]>
2026-06-02 02:39 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Никита Калинин <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Álvaro Herrera @ 2026-06-01 17:12 UTC (permalink / raw)
To: [email protected]; [email protected]; Antonin Houska <[email protected]>; [email protected]
On 2026-May-29, Álvaro Herrera wrote:
> On 2026-05-28, PG Bug reporting form wrote:
>
> > It appears that the pgrepack output plugin is accessible through the
> > SQL logical decoding API, even though the plugin code explicitly
> > indicates that this interface is not supported. Reading changes from
> > such a slot can cause a backend process crash in builds with asserts
> > enabled.
>
> Yeah, I would like to have a way to prevent this, if only for
> user-friendliness, but it's not terribly pressing since only a role
> with REPLICATION privs can create the replication slot, which as I
> recall are already pretty powerful.
How about something like this? It makes your test case throw an error
instead of failing the assertion, which I suppose is an improvement.
The patch is a bit noisy because I moved more code than the minimum
necessary; but the gist of it is that we allocate RepackDecodingState in
repack_startup(), then have repack_setup_logical_decoding() fill in a
magic number, which we later check in repack_begin_txn(). This is a bit
wasteful, because we have to do that check once for each and every
transaction; however I see no other callback that would let us do this
kind of check after the slot is created but before we start to consume
from it.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Before you were born your parents weren't as boring as they are now. They
got that way paying your bills, cleaning up your room and listening to you
tell them how idealistic you are." -- Charles J. Sykes' advice to teenagers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
@ 2026-06-02 02:39 ` Никита Калинин <[email protected]>
2026-06-02 12:25 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Никита Калинин @ 2026-06-02 02:39 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; +Cc: [email protected]; Antonin Houska <[email protected]>; [email protected]
> On 2 Jun 2026, at 00:12, Álvaro Herrera <[email protected]> wrote:
>
> How about something like this? It makes your test case throw an error
> instead of failing the assertion, which I suppose is an improvement.
>
> The patch is a bit noisy because I moved more code than the minimum
> necessary; but the gist of it is that we allocate RepackDecodingState in
> repack_startup(), then have repack_setup_logical_decoding() fill in a
> magic number, which we later check in repack_begin_txn(). This is a bit
> wasteful, because we have to do that check once for each and every
> transaction; however I see no other callback that would let us do this
> kind of check after the slot is created but before we start to consume
> from it.
>
> --
> Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
> "Before you were born your parents weren't as boring as they are now. They
> got that way paying your bills, cleaning up your room and listening to you
> tell them how idealistic you are." -- Charles J. Sykes' advice to teenagers
> <0001-Have-RepackDecodingState-carry-a-magic-number.patch>
Yes, I agree that returning an error to the user makes sense.
But does the error message need to be that detailed? Perhaps something like
"ERROR: wrong magic number in "pgrepack" decoder plugin"
would be sufficient.
Nevertheless, I tested the patch and can confirm that there are no assertion failures anymore.
I also ran it under ASAN and did not observe any issues.
Would it make sense to add a test for this case from the bug report?
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-02 02:39 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Никита Калинин <[email protected]>
@ 2026-06-02 12:25 ` Álvaro Herrera <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Álvaro Herrera @ 2026-06-02 12:25 UTC (permalink / raw)
To: Никита Калинин <[email protected]>; +Cc: [email protected]; Antonin Houska <[email protected]>
Hi Nikita,
On 2026-Jun-02, Никита Калинин wrote:
> > On 2 Jun 2026, at 00:12, Álvaro Herrera <[email protected]> wrote:
> But does the error message need to be that detailed? Perhaps something like
>
> "ERROR: wrong magic number in "pgrepack" decoder plugin"
> would be sufficient.
Maybe. Getting 0x00000000 would be quite different from 0x7f7f7f7f for
instance, or a completely random number, so I don't want to judge ahead
of time.
> Nevertheless, I tested the patch and can confirm that there are no
> assertion failures anymore.
>
> I also ran it under ASAN and did not observe any issues.
Thanks for testing it.
> Would it make sense to add a test for this case from the bug report?
Sure, I would do that.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Saca el libro que tu religión considere como el indicado para encontrar la
oración que traiga paz a tu alma. Luego rebootea el computador
y ve si funciona" (Carlos Duclós)
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
@ 2026-06-03 05:51 ` Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-06-03 05:51 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; +Cc: [email protected]; [email protected]; Antonin Houska <[email protected]>; [email protected]
Hi Álvaro,
On Mon, Jun 1, 2026 at 10:43 PM Álvaro Herrera <[email protected]> wrote:
> On 2026-May-29, Álvaro Herrera wrote:
>
> > On 2026-05-28, PG Bug reporting form wrote:
> >
> > > It appears that the pgrepack output plugin is accessible through the
> > > SQL logical decoding API, even though the plugin code explicitly
> > > indicates that this interface is not supported. Reading changes from
> > > such a slot can cause a backend process crash in builds with asserts
> > > enabled.
> >
> > Yeah, I would like to have a way to prevent this, if only for
> > user-friendliness, but it's not terribly pressing since only a role
> > with REPLICATION privs can create the replication slot, which as I
> > recall are already pretty powerful.
>
> How about something like this? It makes your test case throw an error
> instead of failing the assertion, which I suppose is an improvement.
>
> The patch is a bit noisy because I moved more code than the minimum
> necessary; but the gist of it is that we allocate RepackDecodingState in
> repack_startup(), then have repack_setup_logical_decoding() fill in a
> magic number, which we later check in repack_begin_txn(). This is a bit
> wasteful, because we have to do that check once for each and every
> transaction; however I see no other callback that would let us do this
> kind of check after the slot is created but before we start to consume
> from it.
>
The magic guard is correct. One thing worth noting: the check is in the
begin callback, which fires only at the transaction's commit, so a single
large transaction (a bulk load) is decoded in full and buffered, spilling to
disk past logical_decoding_work_mem, before the plugin rejects it.
That work is then thrown away. It's a misuse path, so this might not be
a big concern, I guess, but it does mean the wasted work scales with
the transaction size rather than being negligible.
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 :)
I attached the patch which brings the above behaviour, this patch in on top
of your patch.
Thoughts?
--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
Attachments:
[application/octet-stream] v1-0002-Reject-the-pgrepack-output-plugin-outside-REPACK-CON.patch (4.4K, 3-v1-0002-Reject-the-pgrepack-output-plugin-outside-REPACK-CON.patch)
download | inline diff:
From 456431823892fe98df3fcfa30d8b8c08006d88e6 Mon Sep 17 00:00:00 2001
From: Srinath Reddy Sadipiralla <[email protected]>
Date: Wed, 3 Jun 2026 10:29:30 +0530
Subject: [PATCH] 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 | 5 +++++
contrib/test_decoding/sql/repack.sql | 4 ++++
src/backend/replication/slotfuncs.c | 14 ++++++++++++++
src/backend/replication/walsender.c | 10 ++++++++++
4 files changed, 33 insertions(+)
diff --git a/contrib/test_decoding/expected/repack.out b/contrib/test_decoding/expected/repack.out
index 6204e620b43..2732561281b 100644
--- a/contrib/test_decoding/expected/repack.out
+++ b/contrib/test_decoding/expected/repack.out
@@ -99,5 +99,10 @@ 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: output plugin "pgrepack" cannot be used through the logical decoding interface
+DETAIL: This output plugin is reserved for internal use 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/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 16fbd383735..17548abf601 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -136,6 +136,20 @@ create_logical_replication_slot(char *name, char *plugin,
Assert(!MyReplicationSlot);
+ /*
+ * The "pgrepack" output plugin is for internal use by REPACK
+ * (CONCURRENTLY) only. Reject it here so that misuse through the SQL
+ * interface fails immediately with a clear message, instead of being
+ * caught later (and only after some decoding work) by the plugin's own
+ * magic-number check in its begin callback. REPACK creates its slot
+ * directly via ReplicationSlotCreate(), so it is unaffected.
+ */
+ if (strcmp(plugin, "pgrepack") == 0)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("output plugin \"pgrepack\" cannot be used through the logical decoding interface"),
+ errdetail("This output plugin is reserved for internal use by REPACK (CONCURRENTLY)."));
+
/*
* Acquire a logical decoding slot, this will check for conflicting names.
* Initially create persistent slot as ephemeral - that allows us to
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 04aa770d981..b9e52a20173 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1270,6 +1270,16 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
CheckLogicalDecodingRequirements(false);
+ /*
+ * The "pgrepack" output plugin is reserved for REPACK (CONCURRENTLY);
+ * reject it here too (see create_logical_replication_slot()).
+ */
+ if (strcmp(cmd->plugin, "pgrepack") == 0)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("output plugin \"pgrepack\" cannot be used through the logical decoding interface"),
+ errdetail("This output plugin is reserved for internal use by REPACK (CONCURRENTLY)."));
+
/*
* Initially create persistent slot as ephemeral - that allows us to
* nicely handle errors during initialization because it'll get
--
2.43.0
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
@ 2026-06-03 07:30 ` Antonin Houska <[email protected]>
2026-06-03 10:25 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 21:02 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Antonin Houska @ 2026-06-03 07:30 UTC (permalink / raw)
To: Srinath Reddy Sadipiralla <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]
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.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
[text/x-diff] repack_reserve_plugin_name.diff (2.3K, 2-repack_reserve_plugin_name.diff)
download | inline diff:
diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index b6b7b604b4f..5213b1e050f 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);
@@ -228,7 +226,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..572bed7b4d1 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,16 @@ 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,
+ 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;
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
@ 2026-06-03 10:25 ` Srinath Reddy Sadipiralla <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-06-03 10:25 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]
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
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
@ 2026-06-03 21:02 ` Alvaro Herrera <[email protected]>
2026-06-04 06:31 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
2026-06-05 06:09 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-05 08:08 ` RE: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Zhijie Hou (Fujitsu) <[email protected]>
1 sibling, 3 replies; 14+ messages in thread
From: Alvaro Herrera @ 2026-06-03 21:02 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: Srinath Reddy Sadipiralla <[email protected]>; [email protected]; [email protected]
On 2026-Jun-03, Antonin Houska 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.
I don't like either of these approaches, because they are forcing the
generic facility (either slot creation or logical decoding setup) to
know something about one specific user of the facility. That is to say,
the restriction is being added on the wrong side of the abstraction.
I know my implementation the drawback you (Srinath) mentioned, because
the abstraction doesn't provide us with a great way to inject an error
report at the exact spot we need it; but I think it's at the correct
side of the abstraction.
(I'm not really sure that there _is_ a great way to throw an error
report at the right time. That would require every single output plugin
author to add a function we can call; and every single one of them,
except REPACK, would do nothing. This seems quite pointless.)
I frankly don't have a problem with letting a transaction spill a few
GBs to disk only to then report an error that pgrepack is being misused.
It's just not something that anyone would do for fun.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
2026-06-03 21:02 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
@ 2026-06-04 06:31 ` Antonin Houska <[email protected]>
2 siblings, 0 replies; 14+ messages in thread
From: Antonin Houska @ 2026-06-04 06:31 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Srinath Reddy Sadipiralla <[email protected]>; [email protected]; [email protected]
Alvaro Herrera <[email protected]> wrote:
> On 2026-Jun-03, Antonin Houska 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.
>
> I don't like either of these approaches, because they are forcing the
> generic facility (either slot creation or logical decoding setup) to
> know something about one specific user of the facility. That is to say,
> the restriction is being added on the wrong side of the abstraction.
> I know my implementation the drawback you (Srinath) mentioned, because
> the abstraction doesn't provide us with a great way to inject an error
> report at the exact spot we need it; but I think it's at the correct
> side of the abstraction.
I noticed that ReplicationSlotAcquire() already does something like that
/*
* Do not allow users to acquire the reserved slot. This scenario may
* occur if the launcher that owns the slot has terminated unexpectedly
* due to an error, and a backend process attempts to reuse the slot.
*/
if (!IsLogicalLauncher() && IsSlotForConflictCheck(name))
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("cannot acquire replication slot \"%s\"", name),
errdetail("The slot is reserved for conflict detection and can only be acquired by logical replication launcher."));
but I agree that it's not perfect to hard-wire particular slot names into
functions like this. Perhaps we could introduce a concept of "reserved slots"
and an API (callback) to perform these checks, but that's not appropriate for
beta release.
> (I'm not really sure that there _is_ a great way to throw an error
> report at the right time. That would require every single output plugin
> author to add a function we can call; and every single one of them,
> except REPACK, would do nothing. This seems quite pointless.)
>
> I frankly don't have a problem with letting a transaction spill a few
> GBs to disk only to then report an error that pgrepack is being misused.
> It's just not something that anyone would do for fun.
I admit that the possibility of wasted processing of a transaction didn't
really frighten me. The idea I posted just occurred to me somehow, but I don't
consider it urgent. I'm fine with your approach.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
2026-06-03 21:02 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
@ 2026-06-05 06:09 ` Srinath Reddy Sadipiralla <[email protected]>
2 siblings, 0 replies; 14+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-06-05 06:09 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Antonin Houska <[email protected]>; [email protected]; [email protected]
On Thu, Jun 4, 2026 at 2:50 AM Alvaro Herrera <[email protected]> wrote:
> On 2026-Jun-03, Antonin Houska 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.
>
> I don't like either of these approaches, because they are forcing the
> generic facility (either slot creation or logical decoding setup) to
> know something about one specific user of the facility. That is to say,
> the restriction is being added on the wrong side of the abstraction.
> I know my implementation the drawback you (Srinath) mentioned, because
> the abstraction doesn't provide us with a great way to inject an error
> report at the exact spot we need it; but I think it's at the correct
> side of the abstraction.
> (I'm not really sure that there _is_ a great way to throw an error
> report at the right time. That would require every single output plugin
> author to add a function we can call; and every single one of them,
> except REPACK, would do nothing. This seems quite pointless.)
>
> I frankly don't have a problem with letting a transaction spill a few
> GBs to disk only to then report an error that pgrepack is being misused.
> It's just not something that anyone would do for fun.
>
makes sense, we can go with your approach, thanks for
the clarification.
--
Thanks :)
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 14+ messages in thread
* RE: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
2026-06-03 21:02 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
@ 2026-06-05 08:08 ` Zhijie Hou (Fujitsu) <[email protected]>
2026-06-08 19:50 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
2 siblings, 1 reply; 14+ messages in thread
From: Zhijie Hou (Fujitsu) @ 2026-06-05 08:08 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; Antonin Houska <[email protected]>; +Cc: Srinath Reddy Sadipiralla <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>
On Thursday, June 4, 2026 5:03 AM Alvaro Herrera <[email protected]> wrote:
> On 2026-Jun-03, Antonin Houska 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.
>
> I don't like either of these approaches, because they are forcing the generic
> facility (either slot creation or logical decoding setup) to know something
> about one specific user of the facility. That is to say, the restriction is being
> added on the wrong side of the abstraction.
> I know my implementation the drawback you (Srinath) mentioned, because
> the abstraction doesn't provide us with a great way to inject an error report at
> the exact spot we need it; but I think it's at the correct side of the abstraction.
I have no objection to the proposed approach. But I would like to confirm
whether reporting an ERROR in the startup callback (when the context is not a
REPACK decoding worker) is considered acceptable.
Like:
repack_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
bool is_init)
...
if (!AmRepackWorker())
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("this plugin can only be used by REPACK (CONCURRENTLY)"));
Best Regards,
Hou zj
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
2026-06-03 21:02 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
2026-06-05 08:08 ` RE: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Zhijie Hou (Fujitsu) <[email protected]>
@ 2026-06-08 19:50 ` Alvaro Herrera <[email protected]>
2026-06-09 18:18 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Alvaro Herrera @ 2026-06-08 19:50 UTC (permalink / raw)
To: Zhijie Hou (Fujitsu) <[email protected]>; +Cc: Antonin Houska <[email protected]>; Srinath Reddy Sadipiralla <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>
On 2026-Jun-05, Zhijie Hou (Fujitsu) wrote:
> I have no objection to the proposed approach. But I would like to confirm
> whether reporting an ERROR in the startup callback (when the context is not a
> REPACK decoding worker) is considered acceptable.
>
> Like:
>
> repack_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
> bool is_init)
> ...
> if (!AmRepackWorker())
> ereport(ERROR,
> errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("this plugin can only be used by REPACK (CONCURRENTLY)"));
Hmm, yeah, that works for me, we can ditch the magic number then. I'm
considering something like the attached. I added the test case and
edited nearby comments. Will stare some more at it tomorrow ...
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Antonin Houska <[email protected]>
2026-06-03 21:02 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
2026-06-05 08:08 ` RE: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Zhijie Hou (Fujitsu) <[email protected]>
2026-06-08 19:50 ` Re: BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API Alvaro Herrera <[email protected]>
@ 2026-06-09 18:18 ` Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Alvaro Herrera @ 2026-06-09 18:18 UTC (permalink / raw)
To: Zhijie Hou (Fujitsu) <[email protected]>; +Cc: Antonin Houska <[email protected]>; Srinath Reddy Sadipiralla <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>
On 2026-Jun-08, Alvaro Herrera wrote:
> Hmm, yeah, that works for me, we can ditch the magic number then. I'm
> considering something like the attached. I added the test case and
> edited nearby comments. Will stare some more at it tomorrow ...
Pushed, thanks everyone.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"¿Cómo puedes confiar en algo que pagas y que no ves,
y no confiar en algo que te dan y te lo muestran?" (Germán Poo)
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2026-06-09 18:18 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-28 14:54 BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API PG Bug reporting form <[email protected]>
2026-05-29 14:23 ` Álvaro Herrera <[email protected]>
2026-06-01 17:12 ` Álvaro Herrera <[email protected]>
2026-06-02 02:39 ` Никита Калинин <[email protected]>
2026-06-02 12:25 ` Álvaro Herrera <[email protected]>
2026-06-03 05:51 ` Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 07:30 ` Antonin Houska <[email protected]>
2026-06-03 10:25 ` Srinath Reddy Sadipiralla <[email protected]>
2026-06-03 21:02 ` Alvaro Herrera <[email protected]>
2026-06-04 06:31 ` Antonin Houska <[email protected]>
2026-06-05 06:09 ` Srinath Reddy Sadipiralla <[email protected]>
2026-06-05 08:08 ` Zhijie Hou (Fujitsu) <[email protected]>
2026-06-08 19:50 ` Alvaro Herrera <[email protected]>
2026-06-09 18:18 ` Alvaro Herrera <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox