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.