Re: Skipping logical replication transactions on subscriber side

On 18.01.22 07:05, Masahiko Sawada wrote:
> I've attached a rebased patch.

I think this is now almost done.  Attached I have a small fixup patch 
with some documentation proof-reading, and removing some comments I felt 
are redundant.  Some others have also sent you some documentation 
updates, so feel free to merge mine in with them.

Some other comments:

parse_subscription_options() and AlterSubscriptionStmt mixes regular 
options and skip options in ways that confuse me.  It seems to work 
correctly, though.  I guess for now it's okay, but if we add more skip 
options, it might be better to separate those more cleanly.

I think the superuser check in AlterSubscription() might no longer be 
appropriate.  Subscriptions can now be owned by non-superusers.  Please 
check that.

The display order in psql \dRs+ is a bit odd.  I would put it at the 
end, certainly not between Two phase commit and Synchronous commit.

Please run pgperltidy over 028_skip_xact.pl.

Is the setting of logical_decoding_work_mem in the test script required? 
  If so, comment why.

Please document arguments origin_lsn and origin_timestamp of
stop_skipping_changes().  Otherwise, one has to dig quite deep to find
out what they are for.

This is all minor stuff, so I think when this and the nearby comments 
are addressed, this is fine by me.