diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index f770e9c..a826be9 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -1079,17 +1079,6 @@ CheckAlterSubOption(Subscription *sub, const char *option, strcmp(option, "two_phase") == 0); /* - * If the slot needs to be updated, the backend must connect to the - * publisher and request the alteration. slot_name must be required at that - * time. - */ - if (slot_needs_update && !sub->slotname) - ereport(ERROR, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("cannot set %s for a subscription that does not have a slot name", - option))); - - /* * Do not allow changing the option if the subscription is enabled. This * is because both failover and two_phase options of the slot on the * publisher cannot be modified if the slot is currently acquired by the @@ -1101,17 +1090,27 @@ CheckAlterSubOption(Subscription *sub, const char *option, errmsg("cannot set %s for enabled subscription", option))); - /* - * The changed option of the slot can't be rolled back, so disallow if we - * are in a transaction block. - */ if (slot_needs_update) { StringInfoData cmd; + /* + * If the slot needs to be updated, the backend must connect to the + * publisher and request the alteration. A slot_name is required at + * that time. + */ + if (!sub->slotname) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot set %s for a subscription that does not have a slot name", + option))); + + /* + * The changed option of the slot can't be rolled back, so disallow if we + * are in a transaction block. + */ initStringInfo(&cmd); appendStringInfo(&cmd, "ALTER SUBSCRIPTION ... SET (%s)", option); - PreventInTransactionBlock(isTopLevel, cmd.data); pfree(cmd.data); } @@ -1132,12 +1131,12 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, HeapTuple tup; Oid subid; bool update_tuple = false; + bool update_failover = false; + bool update_two_phase = false; Subscription *sub; Form_pg_subscription form; bits32 supported_opts; SubOpts opts = {0}; - bool update_failover = false; - bool update_two_phase = false; rel = table_open(SubscriptionRelationId, RowExclusiveLock); @@ -1271,7 +1270,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, if (IsSet(opts.specified_opts, SUBOPT_FAILOVER)) { /* - * First, mark the needs to alter the replication slot. + * First, mark the need to alter the replication slot. * Failover option is controlled by both the publisher (as * a slot option) and the subscriber (as a subscription * option). @@ -1295,15 +1294,14 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, * subscription option). The slot option must be altered * only when changing "true" to "false". * - * There is no need to do something remarkable regarding + * There is no need to do anything remarkable for * the "false" to "true" case; the backend process alters * subtwophase to LOGICALREP_TWOPHASE_STATE_PENDING once. * After the subscription is enabled, a new logical * replication worker requests to change the two_phase * option of its slot from pending to true when the - * initial data synchronization is done. The code path is - * the same as the case in which two_phase is initially - * set to true. + * initial data synchronization is done. That code path is + * the same as when two_phase is initially set to true. */ update_two_phase = !opts.twophase; diff --git a/src/test/subscription/t/021_twophase.pl b/src/test/subscription/t/021_twophase.pl index c8fb3b7..91e2e17 100644 --- a/src/test/subscription/t/021_twophase.pl +++ b/src/test/subscription/t/021_twophase.pl @@ -428,9 +428,8 @@ is($result, qq(0), 'should be no prepared transactions on subscriber'); ############################### # Toggle the two_phase to "true" before the COMMIT PREPARED. -# -# Since we are the special path for the case where both two_phase -# and failover are altered, it is also set to "true". +# Also, set failover to "true" to test the code path where +# both two_phase and failover are altered at the same time. ############################### $node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub_copy DISABLE;");