From 1e8c88c9fb98e15af3a9ffd1bb02559f8216a5c4 Mon Sep 17 00:00:00 2001
From: Nisha Moond <nisha.moond412@gmail.com>
Date: Tue, 3 Jun 2025 10:48:00 +0530
Subject: [PATCH v17 2/2] Fix for pg_dump

When there is subscrption where both two_phase and failover are enabled,
generate the CREATE SUBSCRIPTION command only with two_phase = on and
ignore failover i.e. failover will be false(default).
---
 doc/src/sgml/ref/pg_dump.sgml | 13 +++++++++++++
 src/bin/pg_dump/pg_dump.c     | 12 +++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index cfc74ca6d69..ed077ac4159 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -1633,6 +1633,19 @@ CREATE DATABASE foo WITH TEMPLATE template0;
    had been originally created with <literal>two_phase = true</literal> option.
   </para>
 
+  <para>
+   When dumping logical replication subscriptions where both
+   <link linkend="sql-createsubscription-params-with-two-phase"><literal>two_phase</literal></link>
+   and <link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link>
+   are enabled, <application>pg_dump</application> will generate <command>CREATE
+   SUBSCRIPTION</command> commands with only <literal>two_phase = on</literal>
+   option, ignoring the <literal>failover</literal>. This ensures that restoring
+   the subscription does not fail due to the incompatibility of enabling both
+   options at creation time (see <xref linkend="sql-createsubscription"/>).
+   After the restore, users can enable <literal>failover</literal> using
+   <xref linkend="sql-altersubscription"/>.
+  </para>
+
   <para>
    It is generally recommended to use the <option>-X</option>
    (<option>--no-psqlrc</option>) option when restoring a database from a
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9ed1a856fa3..5c31f30e914 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5129,6 +5129,7 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
 	int			npubnames = 0;
 	int			i;
 	char		two_phase_disabled[] = {LOGICALREP_TWOPHASE_STATE_DISABLED, '\0'};
+	bool		set_failover = true;
 
 	/* Do nothing in data-only dump */
 	if (dopt->dataOnly)
@@ -5174,8 +5175,17 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
 		appendPQExpBufferStr(query, ", streaming = parallel");
 
 	if (strcmp(subinfo->subtwophasestate, two_phase_disabled) != 0)
+	{
 		appendPQExpBufferStr(query, ", two_phase = on");
 
+		/*
+		 * The failover option cannot be set togather with two_phase during
+		 * CREATE SUBSCRIPTION. It can be enabled later using ALTER
+		 * SUBSCRIPTION after the subscription is restored.
+		 */
+		set_failover = false;
+	}
+
 	if (strcmp(subinfo->subdisableonerr, "t") == 0)
 		appendPQExpBufferStr(query, ", disable_on_error = true");
 
@@ -5185,7 +5195,7 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
 	if (strcmp(subinfo->subrunasowner, "t") == 0)
 		appendPQExpBufferStr(query, ", run_as_owner = true");
 
-	if (strcmp(subinfo->subfailover, "t") == 0)
+	if (set_failover && strcmp(subinfo->subfailover, "t") == 0)
 		appendPQExpBufferStr(query, ", failover = true");
 
 	if (strcmp(subinfo->subsynccommit, "off") != 0)
-- 
2.34.1