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

Hi,

On Fri, Sep 18, 2026 at 1:31 AM kedar anavardekar
<kedar(dot)anavardekar(at)gmail(dot)com> wrote:
>
> Two minor naming suggestions: (please take the suggestions if you
> think the points are valid)

Thanks for taking a look.

> 1. Could MyReplicationSlotSubId be renamed to
> MyReplicationSlotSubXactId (or MyReplicationSlotSubTransactionId)
>
> SubId may be read as a subscription ID, whereas this variable stores
> the SubTransactionId of the subtransaction that acquired
> MyReplicationSlot. The more explicit name would make its purpose
> clearer and avoid confusion with logical replication subscriptions.

Subscription and its related replication slot on the publisher are on
two different database instances, and one has the context when reading
the code around this. Also, "SubId" is used across the code base and I
want to keep it consistent and short, so MyReplicationSlotSubId looks
fine to me.

> 2., could the comment above AtEOSubXact_ReplicationSlot() be revised from:
> /*
> * At subxact end, release the replication slot if the subtransaction
> * where the slot was acquired is aborted.
> */
> to:
> /*
> * At subxact end, release the replication slot if the subtransaction
> * in which the slot was acquired is aborted.
> */
> “In which” is more precise here because the slot is acquired during
> that subtransaction.

I believe "where the slot was acquired" is grammatically correct as
well, so I'm fine with the existing wording.

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