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

Hi,

On Fri, Aug 7, 2026 at 4:31 AM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>
> + 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'.

I get your point. Would something like the below work?

+ /*
+ * The aborting subxact is the one that acquired the slot, and its id is
+ * never invalid, so acquiredInSubId is valid here. It is set only when a
+ * slot is held, and cleared when the slot is released, so the slot must
+ * still be held.
+ */
+ Assert(acquiredInSubId != InvalidSubTransactionId);
+ Assert(MyReplicationSlot != NULL);
+ ReplicationSlotRelease();

I removed the unnecessary header file inclusions (review comment from
Shveta upthread) and attached the v11 patch.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com