On Wed, Jan 12, 2022 2:02 PM Masahiko Sawada <sawada.m...@gmail.com> wrote:
>
> I've attached an updated patch that incorporated all comments I got so far.
>
Thanks for updating the patch. Here are some comments:
1)
+ Skip applying changes of the particular transaction. If incoming data
Should "Skip" be "Skips" ?
2)
+ prepared by enabling <literal>two_phase</literal> on susbscriber. After
h
+ the logical replication successfully skips the transaction, the
transaction
The "h" after word "After" seems redundant.
3)
+ Skipping the whole transaction includes skipping the cahnge that may not
violate
"cahnge" should be "changes" I think.
4)
+/*
+ * True if we are skipping all data modification changes (INSERT, UPDATE,
etc.) of
+ * the specified transaction at MySubscription->skipxid. Once we start
skipping
...
+ */
+static TransactionId skipping_xid = InvalidTransactionId;
+#define is_skipping_changes() (TransactionIdIsValid(skipping_xid))
Maybe we should modify this comment. Something like:
skipping_xid is valid if we are skipping all data modification changes ...
5)
+ if (!superuser())
+ ereport(ERROR,
+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must
be superuser to set %s", "skip_xid")));
Should we change the message to "must be superuser to skip xid"?
Because the SQL stmt is "ALTER SUBSCRIPTION ... SKIP (xid = XXX)".
Regards,
Tang
