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

Hi,

On Fri, Aug 7, 2026 at 12:50 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Thu, Aug 6, 2026 at 5:26 AM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> >
> > > Please find the attached v9 patch.
> >
> > The patch looks good overall - just a few quick comments:
>
> Thanks for taking a look at it.
>
> > After releasing the slot in AtEOSubXact_ReplicationSlot(), I'd suggest
> > adding these assertions:
> >
> > Assert(MyReplicationSlot == NULL);
> > Assert(acquiredInSubId == InvalidSubTransactionId);
>
> The slot release function sets MyReplicationSlot to NULL in both the
> ephemeral and the other path, and clears acquiredInSubId with no early
> return, so both conditions already hold there. I would prefer not to
> add asserts that re-check what the release just above guarantees.
>

+ if (MyReplicationSlot != NULL)
+ ReplicationSlotRelease();

From this if-condition in AtEOSubXact_ReplicationSlot(), it appears
that even when 'acquiredInSubId == mySubid', 'MyReplicationSlot' could
be NULL and if that ever happens we may/will return from this function
without clearing 'acquiredInSubId'. That matters because
'currentSubTransactionId' is reset to TopSubTransactionId at each
StartTransaction(), so subxact ids are reused across top-level
transactions; a stale id left behind here could later match an
unrelated subxact.

AFAIU, in practice this should be unreachable: 'acquiredInSubId' is
only ever set together with 'MyReplicationSlot', and both
ReplicationSlotRelease() and ReplicationSlotDropAcquired() clear it,
so "MyReplicationSlot == NULL" implies "acquiredInSubId ==
InvalidSubTransactionId", which can never equal a real `mySubid`. But
the guard's existence suggests you think "MyReplicationSlot == NULL"
is possible.

So either the reasoning above deserves a comment atop the
if-condition, or, if the NULL case really is impossible, an
'Assert(MyReplicationSlot != NULL)' would document it more directly
than a silent 'if'.

--
With Regards,
Ashutosh Sharma.