Hi, Patch v22-0001 LGTM apart from the following nitpicks
======
src/sgml/ref/alter_subscription.sgml
nitpick - /one needs to/you need to/
======
src/backend/commands/subscriptioncmds.c
CheckAlterSubOption:
nitpick = "ideally we could have..." doesn't make sense because the
code uses a more consistent/simpler way. So other option was not ideal
after all.
AlterSubscription
nitpick - typo /syncronization/synchronization/
nipick - plural fix
======
Kind Regards,
Peter Smith.
Fujitsu Australia.
diff --git a/doc/src/sgml/ref/alter_subscription.sgml
b/doc/src/sgml/ref/alter_subscription.sgml
index cbba1ee..6af6d0d 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -272,7 +272,7 @@ ALTER SUBSCRIPTION <replaceable
class="parameter">name</replaceable> RENAME TO <
logical replication worker corresponding to a particular subscription
have
the following pattern: <quote><literal>pg_gid_%u_%u</literal></quote>
(parameters: subscription <parameter>oid</parameter>, remote transaction
id <parameter>xid</parameter>).
- To resolve such transactions manually, one needs to roll back all
+ To resolve such transactions manually, you need to roll back all
the prepared transactions with corresponding subscription IDs in their
names. Applications can check
<link
linkend="view-pg-prepared-xacts"><structname>pg_prepared_xacts</structname></link>
diff --git a/src/backend/commands/subscriptioncmds.c
b/src/backend/commands/subscriptioncmds.c
index 27560f1..b21f5c0 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1090,9 +1090,9 @@ CheckAlterSubOption(Subscription *sub, const char *option,
* publisher cannot be modified if the slot is currently acquired by the
* existing walsender.
*
- * Note that the two_phase is enabled (aka changed from 'false' to
'true')
- * on the publisher by the existing walsender so, ideally, we can allow
- * that even when a subscription is enabled. But we kept this
restriction
+ * Note that two_phase is enabled (aka changed from 'false' to 'true')
+ * on the publisher by the existing walsender, so we could have allowed
+ * that even when the subscription is enabled. But we kept this
restriction
* for the sake of consistency and simplicity.
*/
if (sub->enabled)
@@ -1281,7 +1281,7 @@ AlterSubscription(ParseState *pstate,
AlterSubscriptionStmt *stmt,
* We need to update both the slot and
the subscription
* for two_phase option. We can enable
the two_phase
* option for a slot only once the
initial data
- * syncronization is done. This is to
avoid missing some
+ * synchronization is done. This is to
avoid missing some
* data as explained in comments atop
worker.c.
*/
update_two_phase = !opts.twophase;
@@ -1306,9 +1306,9 @@ AlterSubscription(ParseState *pstate,
AlterSubscriptionStmt *stmt,
*
* Ensure workers have already been
exited to avoid
* getting prepared transactions while
we are disabling
- * two_phase option. Otherwise, the
changes of already
- * prepared transactions can be
replicated again along
- * with its corresponding commit
leading to duplicate data
+ * two_phase option. Otherwise, the
changes of an already
+ * prepared transaction can be
replicated again along
+ * with its corresponding commit,
leading to duplicate data
* or errors.
*/
if (logicalrep_workers_find(subid,
true, true))
