On Thu, Mar 17, 2022 at 8:13 AM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:
>
> On Wed, Mar 16, 2022 4:23 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I've attached an updated version patch.
> >
>
> Thanks for updating the patch. Here are some comments for the v15 patch.
>
> 1. src/backend/replication/logical/worker.c
>
> + * to skip applying the changes when starting to apply changes. The subskiplsn is
> + * cleared after successfully skipping the transaction or applying non-empty
> + * transaction. The latter prevents the mistakenly specified subskiplsn from
>
> Should "applying non-empty transaction" be modified to "finishing a
> transaction"? To be consistent with the description in the
> alter_subscription.sgml.
>
The current wording in the patch seems okay to me as it is good to
emphasize on non-empty transactions.
> 2. src/test/subscription/t/029_on_error.pl
>
> +# Test of logical replication subscription self-disabling feature.
>
> Should we add something about "skip logical replication transactions" in this
> comment?
>
How about: "Tests for disable_on_error and SKIP transaction features."?
I am making some other minor edits in the patch and will take care of
whatever we decide for these comments.
--
With Regards,
Amit Kapila.