On Tue, Oct 12, 2021 at 4:00 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > I've attached updated patches. >
Some comments for the v16-0003 patch: (1) doc/src/sgml/logical-replication.sgml The output from "SELECT * FROM pg_stat_subscription_errors;" still shows "last_failed_time" instead of "last_error_time". doc/src/sgml/ref/alter_subscription.sgml (2) Suggested update (and fix typo: restrited -> restricted): BEFORE: + Setting and resetting of <literal>skip_xid</literal> option is + restrited to superusers. AFTER: + The setting and resetting of the <literal>skip_xid</literal> option is + restricted to superusers. (3) Suggested improvement to the wording: BEFORE: + incoming change or by skipping the whole transaction. This option + specifies transaction ID that logical replication worker skips to + apply. The logical replication worker skips all data modification AFTER: + incoming changes or by skipping the whole transaction. This option + specifies the ID of the transaction whose application is to be skipped + by the logical replication worker. The logical replication worker + skips all data modification (4) src/backend/replication/logical/worker.c Suggested improvement to the comment wording: BEFORE: + * Stop the skipping transaction if enabled. Otherwise, commit the changes AFTER: + * Stop skipping the transaction changes, if enabled. Otherwise, commit the changes (5) skip_xid value validation The validation of the specified skip_xid XID value isn't great. For example, the following value are accepted: ALTER SUBSCRIPTION sub SET (skip_xid='123abcz'); ALTER SUBSCRIPTION sub SET (skip_xid='99$@*'); Regards, Greg Nancarrow Fujitsu Australia