[PATCH] Release replication slot on error in SQL-callable slot functions

On Sun, May 10, 2026 at 5:45 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi Hackers,

SQL-callable replication slot functions acquire a slot (setting
the process-global MyReplicationSlot) but can then ERROR before reaching
ReplicationSlotRelease(). If such an error is caught by a PL/pgSQL
EXCEPTION block (which uses a subtransaction), MyReplicationSlot remains
set because there is no subtransaction-level cleanup hook for replication
slots.

Any subsequent slot operation in the same session then hits
Assert(MyReplicationSlot == NULL) and crashes the backend on assert
enabled builds. In release builds the stale MyReplicationSlot is silently overwritten,
permanently orphaning the old slot as "active." The orphaned slot blocks any other
session from acquiring it, vacuum and WAL deletion.

Repro:

SELECT pg_create_logical_replication_slot('adv_test', 'test_decoding');

DO $$ BEGIN
PERFORM pg_replication_slot_advance('adv_test', '0/1'::pg_lsn);
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('adv_test', NULL, NULL);

2026-05-09 19:45:06.619 UTC [1096805] STATEMENT: SELECT pg_create_logical_replication_slot('adv_test', 'test_decoding');
TRAP: failed Assert("MyReplicationSlot == NULL"), File: "slot.c", Line: 638, PID: 1096805

Attached a patch to address this by wrapping error-prone paths in PG_TRY/PG_CATCH blocks
and call ReplicationSlotRelease().

Thanks for the report and the patch!

I think wrapping the slot-processing code with PG_TRY()/PG_CATCH() seems
a good direction for addressing the issue you reported.

+   PG_CATCH();
+   {
+       ReplicationSlotRelease();

When create_logical_replication_slot() is called with temporary = true,
the created logical replication slot has RS_TEMPORARY persistency. Such a slot
is not dropped by ReplicationSlotRelease(), whereas an RS_EPHEMERAL slot is
dropped via ReplicationSlotDropAcquired().

So even with the v1 patch, a temporary logical replication slot can remain
unexpectedly if pg_create_logical_replication_slot() throws an error.
In this case, should create_logical_replication_slot() explicitly drop the slot
with ReplicationSlotDropAcquired(), or temporarily change the slot persistency
to RS_EPHEMERAL before calling ReplicationSlotRelease()?

Does a newly created logical replication slot created by
pg_copy_logical_replication_slot() have the same issue?

+ PG_CATCH();
+ {
+ ReplicationSlotRelease();

Should ReplicationSlotRelease() be called only when MyReplicationSlot
is not NULL?

/* Acquire the slot so we "own" it */
ReplicationSlotAcquire(NameStr(*slotname), true, true);

-   /* A slot whose restart_lsn has never been reserved cannot be advanced */
-   if (!XLogRecPtrIsValid(MyReplicationSlot->data.restart_lsn))
-       ereport(ERROR,
-               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                errmsg("replication slot \"%s\" cannot be advanced",
-                       NameStr(*slotname)),
-                errdetail("This slot has never previously reserved
WAL, or it has been invalidated.")));
+   PG_TRY();
+   {
+       /* A slot whose restart_lsn has never been reserved cannot be
advanced */
+       if (!XLogRecPtrIsValid(MyReplicationSlot->data.restart_lsn))

Shouldn't ReplicationSlotAcquire() also be moved inside the PG_TRY() block?
Because it can throw an error after setting MyReplicationSlot.

Regards,

--
Fujii Masao

On Mon, 11 May 2026 at 08:31, Fujii Masao <masao.fujii@gmail.com> wrote:

On Sun, May 10, 2026 at 5:45 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi Hackers,

SQL-callable replication slot functions acquire a slot (setting
the process-global MyReplicationSlot) but can then ERROR before reaching
ReplicationSlotRelease(). If such an error is caught by a PL/pgSQL
EXCEPTION block (which uses a subtransaction), MyReplicationSlot remains
set because there is no subtransaction-level cleanup hook for replication
slots.

Any subsequent slot operation in the same session then hits
Assert(MyReplicationSlot == NULL) and crashes the backend on assert
enabled builds. In release builds the stale MyReplicationSlot is silently overwritten,
permanently orphaning the old slot as "active." The orphaned slot blocks any other
session from acquiring it, vacuum and WAL deletion.

Repro:

SELECT pg_create_logical_replication_slot('adv_test', 'test_decoding');

DO $$ BEGIN
PERFORM pg_replication_slot_advance('adv_test', '0/1'::pg_lsn);
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('adv_test', NULL, NULL);

2026-05-09 19:45:06.619 UTC [1096805] STATEMENT: SELECT pg_create_logical_replication_slot('adv_test', 'test_decoding');
TRAP: failed Assert("MyReplicationSlot == NULL"), File: "slot.c", Line: 638, PID: 1096805

Attached a patch to address this by wrapping error-prone paths in PG_TRY/PG_CATCH blocks
and call ReplicationSlotRelease().

Thanks for the report and the patch!

I think wrapping the slot-processing code with PG_TRY()/PG_CATCH() seems
a good direction for addressing the issue you reported.

+   PG_CATCH();
+   {
+       ReplicationSlotRelease();

When create_logical_replication_slot() is called with temporary = true,
the created logical replication slot has RS_TEMPORARY persistency. Such a slot
is not dropped by ReplicationSlotRelease(), whereas an RS_EPHEMERAL slot is
dropped via ReplicationSlotDropAcquired().

So even with the v1 patch, a temporary logical replication slot can remain
unexpectedly if pg_create_logical_replication_slot() throws an error.
In this case, should create_logical_replication_slot() explicitly drop the slot
with ReplicationSlotDropAcquired(), or temporarily change the slot persistency
to RS_EPHEMERAL before calling ReplicationSlotRelease()?

Does a newly created logical replication slot created by
pg_copy_logical_replication_slot() have the same issue?

Additionally pg_logical_slot_get_changes also has the same issue, it
can be reproduced by the following:
SELECT pg_create_logical_replication_slot('test_slot_1', 'test_decoding');

DO $$
BEGIN
-- This will ERROR if the slot_get changes fails for the slot.
PERFORM 1 FROM pg_logical_slot_get_changes('test_slot_1', NULL,
NULL, 'nonexistent-option', 'val');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('test_slot_1', NULL, NULL);

TRAP: failed Assert("MyReplicationSlot == NULL"), File: "slot.c",
Line: 638, PID: 80308
postgres: vignesh postgres [local] SELECT(ExceptionalCondition+0xba)
[0x642e7b2ebae1]
postgres: vignesh postgres [local] SELECT(ReplicationSlotAcquire+0x6e)
[0x642e7b00d732]

Regards,
Vignesh

Hi

On Wed, May 20, 2026 at 11:49 PM vignesh C <vignesh21@gmail.com> wrote:

On Mon, 11 May 2026 at 08:31, Fujii Masao <masao.fujii@gmail.com> wrote:

On Sun, May 10, 2026 at 5:45 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi Hackers,

SQL-callable replication slot functions acquire a slot (setting
the process-global MyReplicationSlot) but can then ERROR before

reaching

ReplicationSlotRelease(). If such an error is caught by a PL/pgSQL
EXCEPTION block (which uses a subtransaction), MyReplicationSlot

remains

set because there is no subtransaction-level cleanup hook for

replication

slots.

Any subsequent slot operation in the same session then hits
Assert(MyReplicationSlot == NULL) and crashes the backend on assert
enabled builds. In release builds the stale MyReplicationSlot is

silently overwritten,

permanently orphaning the old slot as "active." The orphaned slot

blocks any other

session from acquiring it, vacuum and WAL deletion.

Repro:

SELECT pg_create_logical_replication_slot('adv_test', 'test_decoding');

DO $$ BEGIN
PERFORM pg_replication_slot_advance('adv_test', '0/1'::pg_lsn);
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('adv_test', NULL,

NULL);

2026-05-09 19:45:06.619 UTC [1096805] STATEMENT: SELECT

pg_create_logical_replication_slot('adv_test', 'test_decoding');

TRAP: failed Assert("MyReplicationSlot == NULL"), File: "slot.c",

Line: 638, PID: 1096805

Attached a patch to address this by wrapping error-prone paths in

PG_TRY/PG_CATCH blocks

and call ReplicationSlotRelease().

Thanks for the report and the patch!

I think wrapping the slot-processing code with PG_TRY()/PG_CATCH() seems
a good direction for addressing the issue you reported.

+   PG_CATCH();
+   {
+       ReplicationSlotRelease();

When create_logical_replication_slot() is called with temporary = true,
the created logical replication slot has RS_TEMPORARY persistency. Such

a slot

is not dropped by ReplicationSlotRelease(), whereas an RS_EPHEMERAL slot

is

dropped via ReplicationSlotDropAcquired().

So even with the v1 patch, a temporary logical replication slot can

remain

unexpectedly if pg_create_logical_replication_slot() throws an error.
In this case, should create_logical_replication_slot() explicitly drop

the slot

with ReplicationSlotDropAcquired(), or temporarily change the slot

persistency

to RS_EPHEMERAL before calling ReplicationSlotRelease()?

Does a newly created logical replication slot created by
pg_copy_logical_replication_slot() have the same issue?

Additionally pg_logical_slot_get_changes also has the same issue, it
can be reproduced by the following:
SELECT pg_create_logical_replication_slot('test_slot_1', 'test_decoding');

DO $$
BEGIN
-- This will ERROR if the slot_get changes fails for the slot.
PERFORM 1 FROM pg_logical_slot_get_changes('test_slot_1', NULL,
NULL, 'nonexistent-option', 'val');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('test_slot_1', NULL,
NULL);

TRAP: failed Assert("MyReplicationSlot == NULL"), File: "slot.c",
Line: 638, PID: 80308
postgres: vignesh postgres [local] SELECT(ExceptionalCondition+0xba)
[0x642e7b2ebae1]
postgres: vignesh postgres [local] SELECT(ReplicationSlotAcquire+0x6e)
[0x642e7b00d732]

Thank you for letting me know. Fixing these cases in the next update, will
send it shortly.

Thanks,
Satya

Thanks for reporting the issue. I could reproduce the same issue with
all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

thanks
Shveta

Hi

On Fri, May 22, 2026 at 2:16 AM shveta malik <shveta.malik@gmail.com> wrote:

Thanks for reporting the issue. I could reproduce the same issue with
all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

Please find the attached v2 patch that addressed these three cases as well.

Thanks,
Satya

On Mon, May 25, 2026 at 12:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi

On Fri, May 22, 2026 at 2:16 AM shveta malik <shveta.malik@gmail.com> wrote:

Thanks for reporting the issue. I could reproduce the same issue with
all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

Please find the attached v2 patch that addressed these three cases as well.

Thank You for addressuing these cases. A few comments:

1)

+-- Test 2: session remains usable after the error (MyReplicationSlot cleared)

It shoudl be part of 'Test 1' itself and thus should not be named as 'Test 2'

2)
--------
+-- Test 4: copy_replication_slot with max_replication_slots exceeded.
+-- We reduce max_replication_slots artificially by filling all remaining slots.
+-- Instead, trigger an error by copying to an already-existing name.
+DO $$
+BEGIN
+    PERFORM pg_copy_logical_replication_slot('regression_slot_t3',
'regression_slot_t3');
+EXCEPTION WHEN OTHERS THEN
+    RAISE NOTICE 'caught: %', SQLERRM;
+END;
+$$;
+-- The original slot must still exist and be usable
+SELECT count(*) = 1 AS orig_slot_ok FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';
-----------

I don't think we can hit the Assert with above test (at-least I could
not). Since creation of slot itself will fail as the slot with
same-name already exists, MyReplicationSlot will never be set and thus
Assert will not be hit. A better testcase will be below which fails
during LoadOutputPlugin() after slot-creation and MyReplicationSlot is
set already.

SELECT pg_create_logical_replication_slot('src_slot', 'test_decoding');

DO $$
BEGIN
PERFORM pg_copy_logical_replication_slot('src_slot', 'dst_slot',
false, 'nonexistent_plugin');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('src_slot', NULL, NULL);

3)
So overall these are the problematic APIs:

pg_create_logical_replication_slot
pg_replication_slot_advance
pg_copy_logical_replication_slot
pg_logical_slot_peek_binary_changes
pg_logical_slot_peek_changes
pg_logical_slot_get_changes
pg_logical_slot_get_binary_changes

First 3 are are mutually exclusive fixes fow which we have added
testcases. Last 4 are addressed by fixing common function
pg_logical_slot_get_changes_guts(). I think we should add a test case
for at-least any one of these APIs to cover
pg_logical_slot_get_changes_guts().

Thanks.
Shveta

Hi,

On Mon, May 25, 2026 at 2:58 AM shveta malik <shveta.malik@gmail.com> wrote:

On Mon, May 25, 2026 at 12:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi

On Fri, May 22, 2026 at 2:16 AM shveta malik <shveta.malik@gmail.com>

wrote:

Thanks for reporting the issue. I could reproduce the same issue with
all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

Please find the attached v2 patch that addressed these three cases as

well.

Thank You for addressuing these cases. A few comments:

1)

+-- Test 2: session remains usable after the error (MyReplicationSlot
cleared)

It shoudl be part of 'Test 1' itself and thus should not be named as 'Test
2'

2)
--------
+-- Test 4: copy_replication_slot with max_replication_slots exceeded.
+-- We reduce max_replication_slots artificially by filling all remaining
slots.
+-- Instead, trigger an error by copying to an already-existing name.
+DO $$
+BEGIN
+    PERFORM pg_copy_logical_replication_slot('regression_slot_t3',
'regression_slot_t3');
+EXCEPTION WHEN OTHERS THEN
+    RAISE NOTICE 'caught: %', SQLERRM;
+END;
+$$;
+-- The original slot must still exist and be usable
+SELECT count(*) = 1 AS orig_slot_ok FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';
-----------

I don't think we can hit the Assert with above test (at-least I could
not). Since creation of slot itself will fail as the slot with
same-name already exists, MyReplicationSlot will never be set and thus
Assert will not be hit. A better testcase will be below which fails
during LoadOutputPlugin() after slot-creation and MyReplicationSlot is
set already.

SELECT pg_create_logical_replication_slot('src_slot', 'test_decoding');

DO $$
BEGIN
PERFORM pg_copy_logical_replication_slot('src_slot', 'dst_slot',
false, 'nonexistent_plugin');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('src_slot', NULL, NULL);

3)
So overall these are the problematic APIs:

pg_create_logical_replication_slot
pg_replication_slot_advance
pg_copy_logical_replication_slot
pg_logical_slot_peek_binary_changes
pg_logical_slot_peek_changes
pg_logical_slot_get_changes
pg_logical_slot_get_binary_changes

First 3 are are mutually exclusive fixes fow which we have added
testcases. Last 4 are addressed by fixing common function
pg_logical_slot_get_changes_guts(). I think we should add a test case
for at-least any one of these APIs to cover
pg_logical_slot_get_changes_guts().

Thanks for reviewing. Please review the attached v3 patch.

Thanks,
Satya

On Tue, May 26, 2026 at 12:31 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, May 25, 2026 at 2:58 AM shveta malik <shveta.malik@gmail.com> wrote:

On Mon, May 25, 2026 at 12:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi

On Fri, May 22, 2026 at 2:16 AM shveta malik <shveta.malik@gmail.com> wrote:

Thanks for reporting the issue. I could reproduce the same issue with
all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

Please find the attached v2 patch that addressed these three cases as well.

Thank You for addressuing these cases. A few comments:

1)

+-- Test 2: session remains usable after the error (MyReplicationSlot cleared)

It shoudl be part of 'Test 1' itself and thus should not be named as 'Test 2'

2)
--------
+-- Test 4: copy_replication_slot with max_replication_slots exceeded.
+-- We reduce max_replication_slots artificially by filling all remaining slots.
+-- Instead, trigger an error by copying to an already-existing name.
+DO $$
+BEGIN
+    PERFORM pg_copy_logical_replication_slot('regression_slot_t3',
'regression_slot_t3');
+EXCEPTION WHEN OTHERS THEN
+    RAISE NOTICE 'caught: %', SQLERRM;
+END;
+$$;
+-- The original slot must still exist and be usable
+SELECT count(*) = 1 AS orig_slot_ok FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';
-----------

I don't think we can hit the Assert with above test (at-least I could
not). Since creation of slot itself will fail as the slot with
same-name already exists, MyReplicationSlot will never be set and thus
Assert will not be hit. A better testcase will be below which fails
during LoadOutputPlugin() after slot-creation and MyReplicationSlot is
set already.

SELECT pg_create_logical_replication_slot('src_slot', 'test_decoding');

DO $$
BEGIN
PERFORM pg_copy_logical_replication_slot('src_slot', 'dst_slot',
false, 'nonexistent_plugin');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('src_slot', NULL, NULL);

3)
So overall these are the problematic APIs:

pg_create_logical_replication_slot
pg_replication_slot_advance
pg_copy_logical_replication_slot
pg_logical_slot_peek_binary_changes
pg_logical_slot_peek_changes
pg_logical_slot_get_changes
pg_logical_slot_get_binary_changes

First 3 are are mutually exclusive fixes fow which we have added
testcases. Last 4 are addressed by fixing common function
pg_logical_slot_get_changes_guts(). I think we should add a test case
for at-least any one of these APIs to cover
pg_logical_slot_get_changes_guts().

Thanks for reviewing. Please review the attached v3 patch.

A few trivial things:

1)
pg_replication_slot_advance:
+ PG_TRY();
+ {
+ /* Acquire the slot so we "own" it */
+ ReplicationSlotAcquire(NameStr(*slotname), true, true);
+ /* A slot whose restart_lsn has never been reserved cannot be advanced */
+ if (!XLogRecPtrIsValid(MyReplicationSlot->data.restart_lsn))

We can have a blank line after ReplicationSlotAcquire for better readability.

2)

+SELECT 'init' FROM
pg_create_logical_replication_slot('regression_slot_t3',
'test_decoding', true);
+SELECT count(*) = 1 AS slot_exists FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';

The intent is not clear why are we checking existence of
regression_slot_t3? I think we can skip it (or else add a comment if
really needed). The success of previous
pg_create_logical_replication_slot is enough to confirm that session
is healthy to run other slot related queries.

3)
+SELECT pg_drop_replication_slot('regression_slot_phy');
+
+-- cleanup
+SELECT pg_drop_replication_slot('regression_slot_t3');

We can move drop of 'regression_slot_phy' too under '-- cleanup'

~~

I have no further comments other than the trivial things mentioned above.

thanks
Shveta

On Tue, May 26, 2026 at 10:06 AM shveta malik <shveta.malik@gmail.com> wrote:

On Tue, May 26, 2026 at 12:31 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, May 25, 2026 at 2:58 AM shveta malik <shveta.malik@gmail.com> wrote:

On Mon, May 25, 2026 at 12:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi

On Fri, May 22, 2026 at 2:16 AM shveta malik <shveta.malik@gmail.com> wrote:

Thanks for reporting the issue. I could reproduce the same issue with
all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

Please find the attached v2 patch that addressed these three cases as well.

Thank You for addressuing these cases. A few comments:

1)

+-- Test 2: session remains usable after the error (MyReplicationSlot cleared)

It shoudl be part of 'Test 1' itself and thus should not be named as 'Test 2'

2)
--------
+-- Test 4: copy_replication_slot with max_replication_slots exceeded.
+-- We reduce max_replication_slots artificially by filling all remaining slots.
+-- Instead, trigger an error by copying to an already-existing name.
+DO $$
+BEGIN
+    PERFORM pg_copy_logical_replication_slot('regression_slot_t3',
'regression_slot_t3');
+EXCEPTION WHEN OTHERS THEN
+    RAISE NOTICE 'caught: %', SQLERRM;
+END;
+$$;
+-- The original slot must still exist and be usable
+SELECT count(*) = 1 AS orig_slot_ok FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';
-----------

I don't think we can hit the Assert with above test (at-least I could
not). Since creation of slot itself will fail as the slot with
same-name already exists, MyReplicationSlot will never be set and thus
Assert will not be hit. A better testcase will be below which fails
during LoadOutputPlugin() after slot-creation and MyReplicationSlot is
set already.

SELECT pg_create_logical_replication_slot('src_slot', 'test_decoding');

DO $$
BEGIN
PERFORM pg_copy_logical_replication_slot('src_slot', 'dst_slot',
false, 'nonexistent_plugin');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('src_slot', NULL, NULL);

3)
So overall these are the problematic APIs:

pg_create_logical_replication_slot
pg_replication_slot_advance
pg_copy_logical_replication_slot
pg_logical_slot_peek_binary_changes
pg_logical_slot_peek_changes
pg_logical_slot_get_changes
pg_logical_slot_get_binary_changes

First 3 are are mutually exclusive fixes fow which we have added
testcases. Last 4 are addressed by fixing common function
pg_logical_slot_get_changes_guts(). I think we should add a test case
for at-least any one of these APIs to cover
pg_logical_slot_get_changes_guts().

Thanks for reviewing. Please review the attached v3 patch.

A few trivial things:

1)
pg_replication_slot_advance:
+ PG_TRY();
+ {
+ /* Acquire the slot so we "own" it */
+ ReplicationSlotAcquire(NameStr(*slotname), true, true);
+ /* A slot whose restart_lsn has never been reserved cannot be advanced */
+ if (!XLogRecPtrIsValid(MyReplicationSlot->data.restart_lsn))

We can have a blank line after ReplicationSlotAcquire for better readability.

2)

+SELECT 'init' FROM
pg_create_logical_replication_slot('regression_slot_t3',
'test_decoding', true);
+SELECT count(*) = 1 AS slot_exists FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';

The intent is not clear why are we checking existence of
regression_slot_t3? I think we can skip it (or else add a comment if
really needed). The success of previous
pg_create_logical_replication_slot is enough to confirm that session
is healthy to run other slot related queries.

3)
+SELECT pg_drop_replication_slot('regression_slot_phy');
+
+-- cleanup
+SELECT pg_drop_replication_slot('regression_slot_t3');

We can move drop of 'regression_slot_phy' too under '-- cleanup'

~~

I have no further comments other than the trivial things mentioned above.

Missed to inform this earlier, I am not able to apply any version of
the patches shared so far with 'git am'. It gives error, 'patch -p1'
works.

git am v3-0001-Release-replication-slot-on-error-in-slot-SQL-functions.patch
Patch format detection failed.

thanks
Shveta

Hi,

On Mon, May 25, 2026 at 10:50 PM shveta malik <shveta.malik@gmail.com>
wrote:

On Tue, May 26, 2026 at 10:06 AM shveta malik <shveta.malik@gmail.com>
wrote:

On Tue, May 26, 2026 at 12:31 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, May 25, 2026 at 2:58 AM shveta malik <shveta.malik@gmail.com>

wrote:

On Mon, May 25, 2026 at 12:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi

On Fri, May 22, 2026 at 2:16 AM shveta malik <

shveta.malik@gmail.com> wrote:

Thanks for reporting the issue. I could reproduce the same issue

with

all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

Please find the attached v2 patch that addressed these three cases

as well.

Thank You for addressuing these cases. A few comments:

1)

+-- Test 2: session remains usable after the error (MyReplicationSlot

cleared)

It shoudl be part of 'Test 1' itself and thus should not be named as

'Test 2'

2)
--------
+-- Test 4: copy_replication_slot with max_replication_slots exceeded.
+-- We reduce max_replication_slots artificially by filling all

remaining slots.

+-- Instead, trigger an error by copying to an already-existing name.
+DO $$
+BEGIN
+    PERFORM pg_copy_logical_replication_slot('regression_slot_t3',
'regression_slot_t3');
+EXCEPTION WHEN OTHERS THEN
+    RAISE NOTICE 'caught: %', SQLERRM;
+END;
+$$;
+-- The original slot must still exist and be usable
+SELECT count(*) = 1 AS orig_slot_ok FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';
-----------

I don't think we can hit the Assert with above test (at-least I could
not). Since creation of slot itself will fail as the slot with
same-name already exists, MyReplicationSlot will never be set and thus
Assert will not be hit. A better testcase will be below which fails
during LoadOutputPlugin() after slot-creation and MyReplicationSlot is
set already.

SELECT pg_create_logical_replication_slot('src_slot',

'test_decoding');

DO $$
BEGIN
PERFORM pg_copy_logical_replication_slot('src_slot', 'dst_slot',
false, 'nonexistent_plugin');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('src_slot', NULL,

NULL);

3)
So overall these are the problematic APIs:

pg_create_logical_replication_slot
pg_replication_slot_advance
pg_copy_logical_replication_slot
pg_logical_slot_peek_binary_changes
pg_logical_slot_peek_changes
pg_logical_slot_get_changes
pg_logical_slot_get_binary_changes

First 3 are are mutually exclusive fixes fow which we have added
testcases. Last 4 are addressed by fixing common function
pg_logical_slot_get_changes_guts(). I think we should add a test case
for at-least any one of these APIs to cover
pg_logical_slot_get_changes_guts().

Thanks for reviewing. Please review the attached v3 patch.

A few trivial things:

1)
pg_replication_slot_advance:
+ PG_TRY();
+ {
+ /* Acquire the slot so we "own" it */
+ ReplicationSlotAcquire(NameStr(*slotname), true, true);
+ /* A slot whose restart_lsn has never been reserved cannot be advanced

*/

+ if (!XLogRecPtrIsValid(MyReplicationSlot->data.restart_lsn))

We can have a blank line after ReplicationSlotAcquire for better

readability.

2)

+SELECT 'init' FROM
pg_create_logical_replication_slot('regression_slot_t3',
'test_decoding', true);
+SELECT count(*) = 1 AS slot_exists FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';

The intent is not clear why are we checking existence of
regression_slot_t3? I think we can skip it (or else add a comment if
really needed). The success of previous
pg_create_logical_replication_slot is enough to confirm that session
is healthy to run other slot related queries.

3)
+SELECT pg_drop_replication_slot('regression_slot_phy');
+
+-- cleanup
+SELECT pg_drop_replication_slot('regression_slot_t3');

We can move drop of 'regression_slot_phy' too under '-- cleanup'

~~

I have no further comments other than the trivial things mentioned above.

Missed to inform this earlier, I am not able to apply any version of
the patches shared so far with 'git am'. It gives error, 'patch -p1'
works.

git am
v3-0001-Release-replication-slot-on-error-in-slot-SQL-functions.patch
Patch format detection failed.

Thanks , Shveta! Please find the attached v4 patch that addressed your
comments.

Thanks,
Satya

On Tue, May 26, 2026 at 1:41 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, May 25, 2026 at 10:50 PM shveta malik <shveta.malik@gmail.com> wrote:

On Tue, May 26, 2026 at 10:06 AM shveta malik <shveta.malik@gmail.com> wrote:

On Tue, May 26, 2026 at 12:31 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, May 25, 2026 at 2:58 AM shveta malik <shveta.malik@gmail.com> wrote:

On Mon, May 25, 2026 at 12:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi

On Fri, May 22, 2026 at 2:16 AM shveta malik <shveta.malik@gmail.com> wrote:

Thanks for reporting the issue. I could reproduce the same issue with
all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

Please find the attached v2 patch that addressed these three cases as well.

Thank You for addressuing these cases. A few comments:

1)

+-- Test 2: session remains usable after the error (MyReplicationSlot cleared)

It shoudl be part of 'Test 1' itself and thus should not be named as 'Test 2'

2)
--------
+-- Test 4: copy_replication_slot with max_replication_slots exceeded.
+-- We reduce max_replication_slots artificially by filling all remaining slots.
+-- Instead, trigger an error by copying to an already-existing name.
+DO $$
+BEGIN
+    PERFORM pg_copy_logical_replication_slot('regression_slot_t3',
'regression_slot_t3');
+EXCEPTION WHEN OTHERS THEN
+    RAISE NOTICE 'caught: %', SQLERRM;
+END;
+$$;
+-- The original slot must still exist and be usable
+SELECT count(*) = 1 AS orig_slot_ok FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';
-----------

I don't think we can hit the Assert with above test (at-least I could
not). Since creation of slot itself will fail as the slot with
same-name already exists, MyReplicationSlot will never be set and thus
Assert will not be hit. A better testcase will be below which fails
during LoadOutputPlugin() after slot-creation and MyReplicationSlot is
set already.

SELECT pg_create_logical_replication_slot('src_slot', 'test_decoding');

DO $$
BEGIN
PERFORM pg_copy_logical_replication_slot('src_slot', 'dst_slot',
false, 'nonexistent_plugin');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('src_slot', NULL, NULL);

3)
So overall these are the problematic APIs:

pg_create_logical_replication_slot
pg_replication_slot_advance
pg_copy_logical_replication_slot
pg_logical_slot_peek_binary_changes
pg_logical_slot_peek_changes
pg_logical_slot_get_changes
pg_logical_slot_get_binary_changes

First 3 are are mutually exclusive fixes fow which we have added
testcases. Last 4 are addressed by fixing common function
pg_logical_slot_get_changes_guts(). I think we should add a test case
for at-least any one of these APIs to cover
pg_logical_slot_get_changes_guts().

Thanks for reviewing. Please review the attached v3 patch.

A few trivial things:

1)
pg_replication_slot_advance:
+ PG_TRY();
+ {
+ /* Acquire the slot so we "own" it */
+ ReplicationSlotAcquire(NameStr(*slotname), true, true);
+ /* A slot whose restart_lsn has never been reserved cannot be advanced */
+ if (!XLogRecPtrIsValid(MyReplicationSlot->data.restart_lsn))

We can have a blank line after ReplicationSlotAcquire for better readability.

2)

+SELECT 'init' FROM
pg_create_logical_replication_slot('regression_slot_t3',
'test_decoding', true);
+SELECT count(*) = 1 AS slot_exists FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';

The intent is not clear why are we checking existence of
regression_slot_t3? I think we can skip it (or else add a comment if
really needed). The success of previous
pg_create_logical_replication_slot is enough to confirm that session
is healthy to run other slot related queries.

3)
+SELECT pg_drop_replication_slot('regression_slot_phy');
+
+-- cleanup
+SELECT pg_drop_replication_slot('regression_slot_t3');

We can move drop of 'regression_slot_phy' too under '-- cleanup'

~~

I have no further comments other than the trivial things mentioned above.

Missed to inform this earlier, I am not able to apply any version of
the patches shared so far with 'git am'. It gives error, 'patch -p1'
works.

git am v3-0001-Release-replication-slot-on-error-in-slot-SQL-functions.patch
Patch format detection failed.

Thanks , Shveta! Please find the attached v4 patch that addressed your comments.

Thank You for the patch.
I noticed that we are creating regression_slot_t3 as a a temporary
slot, is that intentional? I think creating a permanent slot here will
be a better testcase.

I have made a few cosmetic changes for better readability along with
creating the 'permanent' regression_slot_t3 slot. Please incorporate
what you think is okay. I have no more comments.

thanks
Shveta

Hi Shveta,

On Tue, May 26, 2026 at 8:54 PM shveta malik <shveta.malik@gmail.com> wrote:

On Tue, May 26, 2026 at 1:41 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, May 25, 2026 at 10:50 PM shveta malik <shveta.malik@gmail.com>

wrote:

On Tue, May 26, 2026 at 10:06 AM shveta malik <shveta.malik@gmail.com>

wrote:

On Tue, May 26, 2026 at 12:31 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, May 25, 2026 at 2:58 AM shveta malik <

shveta.malik@gmail.com> wrote:

On Mon, May 25, 2026 at 12:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi

On Fri, May 22, 2026 at 2:16 AM shveta malik <

shveta.malik@gmail.com> wrote:

Thanks for reporting the issue. I could reproduce the same

issue with

all these as well:

pg_logical_slot_peek_changes
pg_logical_slot_get_binary_changes
pg_logical_slot_peek_binary_changes

Please find the attached v2 patch that addressed these three

cases as well.

Thank You for addressuing these cases. A few comments:

1)

+-- Test 2: session remains usable after the error

(MyReplicationSlot cleared)

It shoudl be part of 'Test 1' itself and thus should not be named

as 'Test 2'

2)
--------
+-- Test 4: copy_replication_slot with max_replication_slots

exceeded.

+-- We reduce max_replication_slots artificially by filling all

remaining slots.

+-- Instead, trigger an error by copying to an already-existing

name.

+DO $$
+BEGIN
+    PERFORM pg_copy_logical_replication_slot('regression_slot_t3',
'regression_slot_t3');
+EXCEPTION WHEN OTHERS THEN
+    RAISE NOTICE 'caught: %', SQLERRM;
+END;
+$$;
+-- The original slot must still exist and be usable
+SELECT count(*) = 1 AS orig_slot_ok FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';
-----------

I don't think we can hit the Assert with above test (at-least I

could

not). Since creation of slot itself will fail as the slot with
same-name already exists, MyReplicationSlot will never be set and

thus

Assert will not be hit. A better testcase will be below which

fails

during LoadOutputPlugin() after slot-creation and

MyReplicationSlot is

set already.

SELECT pg_create_logical_replication_slot('src_slot',

'test_decoding');

DO $$
BEGIN
PERFORM pg_copy_logical_replication_slot('src_slot', 'dst_slot',
false, 'nonexistent_plugin');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('src_slot', NULL,

NULL);

3)
So overall these are the problematic APIs:

pg_create_logical_replication_slot
pg_replication_slot_advance
pg_copy_logical_replication_slot
pg_logical_slot_peek_binary_changes
pg_logical_slot_peek_changes
pg_logical_slot_get_changes
pg_logical_slot_get_binary_changes

First 3 are are mutually exclusive fixes fow which we have added
testcases. Last 4 are addressed by fixing common function
pg_logical_slot_get_changes_guts(). I think we should add a test

case

for at-least any one of these APIs to cover
pg_logical_slot_get_changes_guts().

Thanks for reviewing. Please review the attached v3 patch.

A few trivial things:

1)
pg_replication_slot_advance:
+ PG_TRY();
+ {
+ /* Acquire the slot so we "own" it */
+ ReplicationSlotAcquire(NameStr(*slotname), true, true);
+ /* A slot whose restart_lsn has never been reserved cannot be

advanced */

+ if (!XLogRecPtrIsValid(MyReplicationSlot->data.restart_lsn))

We can have a blank line after ReplicationSlotAcquire for better

readability.

2)

+SELECT 'init' FROM
pg_create_logical_replication_slot('regression_slot_t3',
'test_decoding', true);
+SELECT count(*) = 1 AS slot_exists FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';

The intent is not clear why are we checking existence of
regression_slot_t3? I think we can skip it (or else add a comment if
really needed). The success of previous
pg_create_logical_replication_slot is enough to confirm that session
is healthy to run other slot related queries.

3)
+SELECT pg_drop_replication_slot('regression_slot_phy');
+
+-- cleanup
+SELECT pg_drop_replication_slot('regression_slot_t3');

We can move drop of 'regression_slot_phy' too under '-- cleanup'

~~

I have no further comments other than the trivial things mentioned

above.

Missed to inform this earlier, I am not able to apply any version of
the patches shared so far with 'git am'. It gives error, 'patch -p1'
works.

git am

v3-0001-Release-replication-slot-on-error-in-slot-SQL-functions.patch

Patch format detection failed.

Thanks , Shveta! Please find the attached v4 patch that addressed your

comments.

Thank You for the patch.
I noticed that we are creating regression_slot_t3 as a a temporary
slot, is that intentional? I think creating a permanent slot here will
be a better testcase.

No specific reason to use temp slot, ok to create a permanent slot too.

I have made a few cosmetic changes for better readability along with
creating the 'permanent' regression_slot_t3 slot. Please incorporate
what you think is okay. I have no more comments.

Thank you for the changes and review.

Thanks,
Satya

On Wed, May 27, 2026 at 1:31 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Thank you for the changes and review.

When I applied the v4 patch together with Shveta's diff patch and
ran the regression tests, the tests failed.

Could pg_create_physical_replication_slot() still have the same issue
if it throws an error after ReplicationSlotCreate() and that error is
caught by a PL/pgSQL EXCEPTION block?

Also, do maybe pg_copy_physical_replication_slot(), pg_drop_replication_slot(),
and ALTER_REPLICATION_SLOT potentially have the same issue as well?

Regards,

--
Fujii Masao

On Wed, May 27, 2026 at 1:42 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Wed, May 27, 2026 at 1:31 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Thank you for the changes and review.

When I applied the v4 patch together with Shveta's diff patch and
ran the regression tests, the tests failed.

That is because my top-up patch lacks slot.out changes, I wanted Satya
to first confirm if the changes are acceptable to him. Attached
another top-up patch for test-output correction.

Could pg_create_physical_replication_slot() still have the same issue
if it throws an error after ReplicationSlotCreate() and that error is
caught by a PL/pgSQL EXCEPTION block?

Also, do maybe pg_copy_physical_replication_slot(), pg_drop_replication_slot(),
and ALTER_REPLICATION_SLOT potentially have the same issue as well?

pg_copy_physical_replication_slot() should not have it as the common
'copy_replication_slot' is already fixed in the patch. I will review
the others.

thanks
Shveta

On Wednesday, May 27, 2026 7:00 PM shveta malik <shveta.malik@gmail.com> wrote:

On Wed, May 27, 2026 at 1:42 PM Fujii Masao <masao.fujii@gmail.com>
wrote:

On Wed, May 27, 2026 at 1:31 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Thank you for the changes and review.

Could pg_create_physical_replication_slot() still have the same issue
if it throws an error after ReplicationSlotCreate() and that error is
caught by a PL/pgSQL EXCEPTION block?

Also, do maybe pg_copy_physical_replication_slot(),
pg_drop_replication_slot(), and ALTER_REPLICATION_SLOT potentially have

the same issue as well?

pg_copy_physical_replication_slot() should not have it as the common
'copy_replication_slot' is already fixed in the patch. I will review the others.

I have one slight concern about the approach of releasing the slot within a
PG_CATCH() block in lots of functions. I'm not entirely sure if it's safe or
acceptable to do so before aborting the current transaction, so just to confirm
it once:

Since both ReplicationSlotRelease() and ReplicationSlotDropPtr() acquire
LWLocks, it's possible that a backend reports an ERROR while already holding
one of these locks, then enters the PG_CATCH() block and calls
ReplicationSlotRelease(), which attempts to acquire the same LWLock. However,
LWLocks do not distinguish between locks held by the same backend versus other
backends, so the backend could block forever and become uninterruptible.

I don't have a better alternative, but I think we can
evaluate once whether this is a real risk and if it's acceptable (perhaps the
scenario is rare enough to be acceptable). It may also be worth adding comments
to document this risk.

Best Regards,
Hou zj

On Wed, May 27, 2026 at 8:00 PM shveta malik <shveta.malik@gmail.com> wrote:

pg_copy_physical_replication_slot() should not have it as the common
'copy_replication_slot' is already fixed in the patch.

copy_replication_slot() calls create_physical_replication_slot() before
entering the PG_TRY/PG_CATCH block. So if create_physical_replication_slot()
throws an error, wouldn't the same issue still occur?

I will review
the others.

Thanks!

Regards,

--
Fujii Masao

Hi,

On Wed, May 27, 2026 at 1:12 AM Fujii Masao <masao.fujii@gmail.com> wrote:

On Wed, May 27, 2026 at 1:31 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Thank you for the changes and review.

When I applied the v4 patch together with Shveta's diff patch and
ran the regression tests, the tests failed.

Could pg_create_physical_replication_slot() still have the same issue
if it throws an error after ReplicationSlotCreate() and that error is
caught by a PL/pgSQL EXCEPTION block

Also, do maybe pg_copy_physical_replication_slot(),
pg_drop_replication_slot(),
and ALTER_REPLICATION_SLOT potentially have the same issue as well?

Addressed these in v5 patch, will send out shortly. ALTER_REPLICATION_SLOT
is not exploitable by a SQL query though it has a similar signature.
A walsender error terminates the session so there is no session to leave in
a bad state.

Hi

On Wed, May 27, 2026 at 4:00 AM shveta malik <shveta.malik@gmail.com> wrote:

On Wed, May 27, 2026 at 1:42 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Wed, May 27, 2026 at 1:31 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Thank you for the changes and review.

When I applied the v4 patch together with Shveta's diff patch and
ran the regression tests, the tests failed.

That is because my top-up patch lacks slot.out changes, I wanted Satya
to first confirm if the changes are acceptable to him. Attached
another top-up patch for test-output correction.

Thanks for the patches, I combined these changes in my latest patch. Please
find the v5.

Thanks,
Satya

On Thu, May 28, 2026 at 10:11 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Thanks for the patches, I combined these changes in my latest patch. Please find the v5.

Thanks for updating the patch! But, v5 patch caused a compilation failure.

slotfuncs.c:119:32: error: too few arguments to function call, single
argument 'try_disable' was not specified
119 | ReplicationSlotDropAcquired();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
../../../src/include/replication/slot.h:338:13: note:
'ReplicationSlotDropAcquired' declared here
338 | extern void ReplicationSlotDropAcquired(bool try_disable);
| ^ ~~~~~~~~~~~~~~~~
slotfuncs.c:207:32: error: too few arguments to function call, single
argument 'try_disable' was not specified
207 | ReplicationSlotDropAcquired();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
../../../src/include/replication/slot.h:338:13: note:
'ReplicationSlotDropAcquired' declared here
338 | extern void ReplicationSlotDropAcquired(bool try_disable);
| ^ ~~~~~~~~~~~~~~~~
slotfuncs.c:922:32: error: too few arguments to function call, single
argument 'try_disable' was not specified
922 | ReplicationSlotDropAcquired();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
../../../src/include/replication/slot.h:338:13: note:
'ReplicationSlotDropAcquired' declared here
338 | extern void ReplicationSlotDropAcquired(bool try_disable);
| ^ ~~~~~~~~~~~~~~~~
3 errors generated.

Regards,

--
Fujii Masao