On Mon, Jan 17, 2022 2:18 PM Masahiko Sawada <sawada.m...@gmail.com> wrote:
>
> I've attached an updated patch. Please review it.
>
Thanks for updating the patch. Few comments:
1)
/* Two_phase is only supported in v15 and higher */
if (pset.sversion >= 150000)
appendPQExpBuffer(&buf,
- ", subtwophasestate
AS \"%s\"\n",
- gettext_noop("Two
phase commit"));
+ ", subtwophasestate
AS \"%s\"\n"
+ ", subskipxid AS
\"%s\"\n",
+ gettext_noop("Two
phase commit"),
+ gettext_noop("Skip
XID"));
appendPQExpBuffer(&buf,
", subsynccommit AS \"%s\"\n"
I think "skip xid" should be mentioned in the comment. Maybe it could be
changed to:
"Two_phase and skip XID are only supported in v15 and higher"
2) The following two places are not consistent in whether "= value" is surround
with square brackets.
+ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> SKIP (
<replaceable class="parameter">skip_option</replaceable> [= <replaceable
class="parameter">value</replaceable>] [, ... ] )
+ <term><literal>SKIP ( <replaceable
class="parameter">skip_option</replaceable> = <replaceable
class="parameter">value</replaceable> [, ... ] )</literal></term>
Should we modify the first place to:
+ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> SKIP (
<replaceable class="parameter">skip_option</replaceable> = <replaceable
class="parameter">value</replaceable> [, ... ] )
Because currently there is only one skip_option - xid, and a parameter must be
specified when using it.
3)
+ * Protect subskip_xid of pg_subscription from being concurrently
updated
+ * while clearing it.
"subskip_xid" should be "subskipxid" I think.
4)
+/*
+ * Start skipping changes of the transaction if the given XID matches the
+ * transaction ID specified by skip_xid option.
+ */
The option name was "skip_xid" in the previous version, and it is "xid" in
latest patch. So should we modify "skip_xid option" to "skip xid option", or
"skip option xid", or something else?
Also the following place has similar issue:
+ * the subscription if hte user has specified skip_xid. Once we start skipping
Regards,
Tang
