v2-0001-Report-set-parameters-on-recovery_target-conflict.patch

From 9b96e0f327d7ca2f644b259511776ef7286ec72f Mon Sep 17 00:00:00 2001
From: Scott Ray <scott@scottray.io>
Date: Sun, 31 May 2026 13:12:29 -0700
Subject: [PATCH v2] Report set parameters on recovery_target conflict; expand
 tests

v4 of "Don't call ereport(ERROR) from recovery target GUC assign
hooks" produces a FATAL with an errdetail that lists all five
recovery_target_* GUCs regardless of which the operator actually
set, and exercises only recovery_target_xid in the cleared-then-set
direction.

This patch appends the names and values of the GUCs that are
actually non-empty as a trailing sentence in the existing errdetail.
The "at most one of [list]" enumeration stays in errdetail and no
errhint is emitted, matching the archive_command / archive_library
precedent in pgarch.c.

The TAP test gains four cleared-then-set cases covering time, name,
lsn, and the bare recovery_target, mirroring the existing xid case.
A new like() assertion verifies that the errdetail names which GUCs
are set and their values, using quotemeta on the expected values.

Applies atop v4.

Discussion: https://postgr.es/m/CACSdjfPUa4UvKjADgOERXoxNYmCg2mqqiqKkiJk6mX6E4qgVFw@mail.gmail.com
---
 src/backend/access/transam/xlogrecovery.c   | 35 ++++++++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 40 +++++++++++++++++++++
 2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 1253bed1058..6af36960ddc 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1167,31 +1167,58 @@ validateRecoveryParameters(void)
  * assign hooks must never fail.  Moving the check here keeps the assign hooks
  * contract-compliant.
  *
- * If a future patch adds a sixth recovery_target_* GUC, both this list and
- * the errdetail below must be updated.
+ * If a future patch adds a sixth recovery_target_* GUC, add another
+ * GetConfigOption block below to include it in the "Currently set:"
+ * suffix, and extend the fixed enumeration in the errdetail.
  */
 static void
 CheckRecoveryTargetConflicts(void)
 {
+	StringInfoData set_targets;
 	int			ntargets = 0;
 	const char *val;
 
+	initStringInfo(&set_targets);
+
 	/* missing_ok=false guarantees val is non-NULL. */
 	val = GetConfigOption("recovery_target", false, false);
 	if (val[0] != '\0')
+	{
+		appendStringInfo(&set_targets, "\"recovery_target\" set to \"%s\"", val);
 		ntargets++;
+	}
 	val = GetConfigOption("recovery_target_lsn", false, false);
 	if (val[0] != '\0')
+	{
+		if (ntargets > 0)
+			appendStringInfoString(&set_targets, ", ");
+		appendStringInfo(&set_targets, "\"recovery_target_lsn\" set to \"%s\"", val);
 		ntargets++;
+	}
 	val = GetConfigOption("recovery_target_name", false, false);
 	if (val[0] != '\0')
+	{
+		if (ntargets > 0)
+			appendStringInfoString(&set_targets, ", ");
+		appendStringInfo(&set_targets, "\"recovery_target_name\" set to \"%s\"", val);
 		ntargets++;
+	}
 	val = GetConfigOption("recovery_target_time", false, false);
 	if (val[0] != '\0')
+	{
+		if (ntargets > 0)
+			appendStringInfoString(&set_targets, ", ");
+		appendStringInfo(&set_targets, "\"recovery_target_time\" set to \"%s\"", val);
 		ntargets++;
+	}
 	val = GetConfigOption("recovery_target_xid", false, false);
 	if (val[0] != '\0')
+	{
+		if (ntargets > 0)
+			appendStringInfoString(&set_targets, ", ");
+		appendStringInfo(&set_targets, "\"recovery_target_xid\" set to \"%s\"", val);
 		ntargets++;
+	}
 
 	if (ntargets > 1)
 		ereport(FATAL,
@@ -1201,7 +1228,9 @@ CheckRecoveryTargetConflicts(void)
 						   "\"recovery_target_lsn\", "
 						   "\"recovery_target_name\", "
 						   "\"recovery_target_time\", "
-						   "\"recovery_target_xid\" can be set.")));
+						   "\"recovery_target_xid\" can be set. "
+						   "Currently set: %s.",
+						   set_targets.data)));
 }
 
 /*
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 5979663b0ab..f00a82d4e9e 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -282,6 +282,14 @@ like(
 	qr/multiple recovery targets specified/,
 	'expected error message logged without recovery.signal');
 
+# Ordering in the errdetail follows the GetConfigOption sequence in
+# CheckRecoveryTargetConflicts: recovery_target, recovery_target_lsn,
+# recovery_target_name, recovery_target_time, recovery_target_xid.
+like(
+	$logfile_no_signal,
+	qr/Currently set: "recovery_target_name" set to "\Q$recovery_name\E", "recovery_target_time" set to "\Q$recovery_time\E"/,
+	'errdetail names which recovery_target_* GUCs are set and their values');
+
 # Same-GUC last-wins (one source of truth for the GUC's value): assigning a
 # recovery_target_* GUC and then assigning the same GUC to an empty string
 # leaves no target set and recovery proceeds to the end of WAL.  This is the
@@ -358,6 +366,38 @@ is($count_xid_clear_set, "2000",
 	'recovery_target_xid honored when cleared then set');
 $node_xid_clear_set->teardown_node;
 
+test_recovery_standby_with_options(
+	'recovery_target_time cleared then set',
+	'standby_time_clear_set',
+	$node_primary,
+	"-c recovery_target_time= -c recovery_target_time=$recovery_time_t",
+	"3000",
+	$lsn3);
+
+test_recovery_standby_with_options(
+	'recovery_target_name cleared then set',
+	'standby_name_clear_set',
+	$node_primary,
+	"-c recovery_target_name= -c recovery_target_name=$recovery_name",
+	"4000",
+	$lsn4);
+
+test_recovery_standby_with_options(
+	'recovery_target_lsn cleared then set',
+	'standby_lsn_clear_set',
+	$node_primary,
+	"-c recovery_target_lsn= -c recovery_target_lsn=$recovery_lsn",
+	"5000",
+	$lsn5);
+
+test_recovery_standby_with_options(
+	'recovery_target cleared then set',
+	'standby_immediate_clear_set',
+	$node_primary,
+	"-c recovery_target= -c recovery_target=immediate",
+	"1000",
+	$lsn1);
+
 # Invalid recovery_target_timeline tests
 my ($result, $stdout, $stderr) = $node_primary->psql('postgres',
 	"ALTER SYSTEM SET recovery_target_timeline TO 'bogus'");
-- 
2.50.1 (Apple Git-155)