On Mon, Mar 31, 2025 at 5:04 PM Zhijie Hou (Fujitsu)
<houzj.f...@fujitsu.com> wrote:
>
> Thanks for the comments, they have been addressed in V2.
>
In the PG17 patch, we only have a check preventing failover and
two_phase in CreateSubscription. Don't we need a similar check for
AlterSubscription?
Apart from the above, I have a few suggestions for changes in docs,
error messages, and comments for both versions. See attached.
--
With Regards,
Amit Kapila.
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 141c140331d..bff0d143ac5 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2566,7 +2566,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
</para>
<para>
The address (<literal>LSN</literal>) from which the decoding of prepared
- transactions is enabled. Always <literal>NULL</literal> for physical
+ transactions is enabled. <literal>NULL</literal> for physical
slots.
</para></entry>
</row>
diff --git a/src/test/recovery/t/040_standby_failover_slots_sync.pl
b/src/test/recovery/t/040_standby_failover_slots_sync.pl
index 67cc6374565..19273a49914 100644
--- a/src/test/recovery/t/040_standby_failover_slots_sync.pl
+++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl
@@ -896,9 +896,9 @@ is($result, 't',
# Promote the standby1 to primary. Confirm that:
# a) the slot 'lsub1_slot' and 'snap_test_slot' are retained on the new primary
# b) logical replication for regress_mysub1 is resumed successfully after
failover
-# c) changes from the transaction 'test_twophase_slotsync', which was prepared
-# on the old primary, can be consumed from the synced slot 'snap_test_slot'
-# once committed on the new primary.
+# c) changes from the transaction prepared 'test_twophase_slotsync' can be
+# consumed from the synced slot 'snap_test_slot' once committed on the new
+# primary.
# d) changes can be consumed from the synced slot 'snap_test_slot'
##################################################
$primary->wait_for_replay_catchup($standby1);
diff --git a/src/backend/commands/subscriptioncmds.c
b/src/backend/commands/subscriptioncmds.c
index 8308ccaad5a..df7ab827f7d 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -657,7 +657,7 @@ CreateSubscription(ParseState *pstate,
CreateSubscriptionStmt *stmt,
if (opts.twophase && opts.failover)
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot enable failover option when
two_phase option is enabled"));
+ errmsg("failover and two_phase are mutually
exclusive options"));
/*
* If built with appropriate switch, whine when regression-testing
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index c1bbd16254e..da510f60f9c 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -357,7 +357,7 @@ ReplicationSlotCreate(const char *name, bool db_specific,
if (two_phase)
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot enable failover for a
replication slot that enables two-phase decoding"));
+ errmsg("failover and two_phase are
mutually exclusive options"));
}
/*
@@ -873,7 +873,7 @@ ReplicationSlotAlter(const char *name, bool failover)
if (failover && MyReplicationSlot->data.two_phase)
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot enable failover for a
replication slot that enables two-phase decoding"));
+ errmsg("cannot enable failover for a two-phase
enabled replication slot"));
if (MyReplicationSlot->data.failover != failover)
{
