Hi,
On Fri, Sep 18, 2026 at 11:37 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> I reviewed the v17 patch and here are some comments:
Thanks for taking a look at it.
> for use by the current session. Temporary slots are also
> - released upon any error. This function corresponds
> + dropped on any error. An error raised and caught in a
> + subtransaction, for example by a
> + <application>PL/pgSQL</application> exception block, does not
> + drop them. This function corresponds
>
> ISTM what the following sentence says seems to contradict with what
> the first sentence says. How about rephrasing it to:
>
> for use by the current session. Temporary slots are also
> - released upon any error. This function corresponds
> + dropped when an error is reported to the client. An error
> caught inside a
> + subtransaction, for example by a <application>PL/pgSQL</application>
> + exception block, does not drop them. This function corresponds
I think "when an error is reported to the client" may not be apt for
all cases here. For example, a temporary slot could be created and
errored out by a worker internally using SPI interface, not
necessarily by a client. I don't want that confusion, so I dropped the
client part.
> ---
> +-- A slot function that errors out must still release the slot, otherwise the
> +-- next slot operation in the session fails an assertion or leaks the slot.
> +-- Advancing a freshly created slot to a low LSN always errors.
> +SELECT 'init' FROM
> pg_create_logical_replication_slot('regress_subxact_slot',
> 'test_decoding');
>
> The comment seems not to be in the right place; it's in right before
> the pg_create_logical_replication_slot() call but not related. Given
> that we have the comments for subsequent tests, we can remove it.
My intention was to have a comment describing the group of tests, like
the other groups in this test file have. I slightly adjusted it to
match those.
> I've attached the updated patch that incorporated the above comments.
> I'm going to push it early next week, barring any objections.
Please find the attached v18 patches (incl. backbranches) with the
above two slight adjustments. I fixed a typo in the commit message but
otherwise retained it as-is.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
