On Thu, Mar 17, 2022 at 8:13 AM shiy.f...@fujitsu.com <shiy.f...@fujitsu.com> wrote: > > On Wed, Mar 16, 2022 4:23 PM Masahiko Sawada <sawada.m...@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.
