From 929515539b0919d560088f97d89b02376ca5492b Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Date: Mon, 8 Apr 2024 12:39:12 +0000
Subject: [PATCH v3 3/5] Prohibit altering from true to false if there are
 prepared transactions on subscriber

---
 doc/src/sgml/ref/alter_subscription.sgml |  8 +++++++-
 src/backend/access/transam/twophase.c    |  4 +++-
 src/backend/commands/subscriptioncmds.c  | 13 ++++++++++---
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 20b45e36e0..4f33769858 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -231,7 +231,6 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
       <link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link>, and
       <link linkend="sql-createsubscription-params-with-two-phase"><literal>two_phase</literal></link>.
       Only a superuser can set <literal>password_required = false</literal>.
-      <literal>two_phase</literal> can be altered only for disabled subscription.
      </para>
 
      <para>
@@ -253,6 +252,13 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
       <link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link>
       option is enabled.
      </para>
+
+     <para>
+      <literal>two_phase</literal> can be altered only for disabled
+      subscriptions.  Prepared transactions done by the logical replication
+      worker must not be existed. If found, the <command>ALTER
+      SUBSCRIPTION</command> command will fail.
+     </para>
     </listitem>
    </varlistentry>
 
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 495f99a357..34bf6bfb0b 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -2703,7 +2703,7 @@ checkGid(char *gid, Oid subid)
 
 /*
  * LookupGXactBySubid
- *		Check if the prepared transaction done by apply worker exists.
+ *      Check if the prepared transaction done by the given subscription.
  */
 bool
 LookupGXactBySubid(Oid subid)
@@ -2721,7 +2721,9 @@ LookupGXactBySubid(Oid subid)
 			found = true;
 			break;
 		}
+
 	}
 	LWLockRelease(TwoPhaseStateLock);
+
 	return found;
 }
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index bfbb2873b1..563c757be5 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1188,13 +1188,20 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
 								 errmsg("cannot set %s for enabled subscription",
 										"two_phase")));
 
-					/* Check whether the number of prepared transactions */
+					/*
+					 * If the two_phase is altered from true to false,
+					 * prepared transactions shipped from the publisher won't
+					 * be resolved anymore. Therefore, reject the ALTER
+					 * command if they exists.
+					 */
 					if (!opts.twophase &&
 						form->subtwophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED &&
-						LookupGXactBySubid(subid))
+						LookupGXactBySubid(form->oid))
 						ereport(ERROR,
 								(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-								 errmsg("cannot disable two_phase when uncommitted prepared transactions present")));
+								 errmsg("cannot alter %s to false if there are prepared transactions by the subscription",
+										"two_phase")));
+
 
 					/* Change system catalog acoordingly */
 					values[Anum_pg_subscription_subtwophasestate - 1] =
-- 
2.43.0