BUG #19500: pgrepack logical decoding plugin can crash assert builds via SQL decoding API

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

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

On 2 Jun 2026, at 00:12, Álvaro Herrera <alvherre@kurilemu.de> 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?

Hi Nikita,

On 2026-Jun-02, Никита Калинин wrote:

On 2 Jun 2026, at 00:12, Álvaro Herrera <alvherre@kurilemu.de> 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)

Hi Álvaro,

On Mon, Jun 1, 2026 at 10:43 PM Álvaro Herrera <alvherre@kurilemu.de> 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/

Srinath Reddy Sadipiralla <srinath2133@gmail.com> 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

On Wed, Jun 3, 2026 at 1:00 PM Antonin Houska <ah@cybertec.at> wrote:

Srinath Reddy Sadipiralla <srinath2133@gmail.com> 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/

On 2026-Jun-03, Antonin Houska wrote:

Srinath Reddy Sadipiralla <srinath2133@gmail.com> 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/

Alvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-Jun-03, Antonin Houska wrote:

Srinath Reddy Sadipiralla <srinath2133@gmail.com> 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

On Thu, Jun 4, 2026 at 2:50 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-Jun-03, Antonin Houska wrote:

Srinath Reddy Sadipiralla <srinath2133@gmail.com> 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/

On Thursday, June 4, 2026 5:03 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-Jun-03, Antonin Houska wrote:

Srinath Reddy Sadipiralla <srinath2133@gmail.com> 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

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/

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)