Hi

> The attached seems to be the simplest way to fix this. (Needs
> documentation updates, test updates, error message refinement.)

Thank you!
I add documentation change, tests and rephrased error message.

regards, Sergei
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index db1a2d4..7cbea6e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3221,7 +3221,7 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
       <varname>recovery_target_lsn</varname>, <varname>recovery_target_name</varname>,
       <varname>recovery_target_time</varname>, or <varname>recovery_target_xid</varname>
       can be used; if more than one of these is specified in the configuration
-      file, the last entry will be used.
+      file, a error will be raised.
       These parameters can only be set at server start.
      </para>
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6497393..290a157 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -11065,14 +11065,11 @@ check_recovery_target(char **newval, void **extra, GucSource source)
 static void
 assign_recovery_target(const char *newval, void *extra)
 {
+	if (recoveryTarget)
+		elog(ERROR, "can not specify multiple recovery targets");
+
 	if (newval && strcmp(newval, "") != 0)
 		recoveryTarget = RECOVERY_TARGET_IMMEDIATE;
-	else
-		/*
-		 * Reset recoveryTarget to RECOVERY_TARGET_UNSET to proper handle user
-		 * setting multiple recovery_target with blank value on last.
-		 */
-		recoveryTarget = RECOVERY_TARGET_UNSET;
 }
 
 static bool
@@ -11098,13 +11095,14 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source)
 static void
 assign_recovery_target_xid(const char *newval, void *extra)
 {
+	if (recoveryTarget)
+		elog(ERROR, "can not specify multiple recovery targets");
+
 	if (newval && strcmp(newval, "") != 0)
 	{
 		recoveryTarget = RECOVERY_TARGET_XID;
 		recoveryTargetXid = *((TransactionId *) extra);
 	}
-	else
-		recoveryTarget = RECOVERY_TARGET_UNSET;
 }
 
 static bool
@@ -11161,13 +11159,14 @@ check_recovery_target_time(char **newval, void **extra, GucSource source)
 static void
 assign_recovery_target_time(const char *newval, void *extra)
 {
+	if (recoveryTarget)
+		elog(ERROR, "can not specify multiple recovery targets");
+
 	if (newval && strcmp(newval, "") != 0)
 	{
 		recoveryTarget = RECOVERY_TARGET_TIME;
 		recoveryTargetTime = *((TimestampTz *) extra);
 	}
-	else
-		recoveryTarget = RECOVERY_TARGET_UNSET;
 }
 
 static bool
@@ -11186,13 +11185,14 @@ check_recovery_target_name(char **newval, void **extra, GucSource source)
 static void
 assign_recovery_target_name(const char *newval, void *extra)
 {
+	if (recoveryTarget)
+		elog(ERROR, "can not specify multiple recovery targets");
+
 	if (newval && strcmp(newval, "") != 0)
 	{
 		recoveryTarget = RECOVERY_TARGET_NAME;
 		recoveryTargetName = (char *) newval;
 	}
-	else
-		recoveryTarget = RECOVERY_TARGET_UNSET;
 }
 
 static bool
@@ -11240,13 +11240,14 @@ check_recovery_target_lsn(char **newval, void **extra, GucSource source)
 static void
 assign_recovery_target_lsn(const char *newval, void *extra)
 {
+	if (recoveryTarget)
+		elog(ERROR, "can not specify multiple recovery targets");
+
 	if (newval && strcmp(newval, "") != 0)
 	{
 		recoveryTarget = RECOVERY_TARGET_LSN;
 		recoveryTargetLSN = *((XLogRecPtr *) extra);
 	}
-	else
-		recoveryTarget = RECOVERY_TARGET_UNSET;
 }
 
 static bool
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index f3a7ba4..1fd4aec 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -79,7 +79,7 @@ extern HotStandbyState standbyState;
  */
 typedef enum
 {
-	RECOVERY_TARGET_UNSET,
+	RECOVERY_TARGET_UNSET = 0,
 	RECOVERY_TARGET_XID,
 	RECOVERY_TARGET_TIME,
 	RECOVERY_TARGET_NAME,
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index f6f2e8b..cba367c 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 9;
+use Test::More tests => 10;
 
 # Create and test a standby from given backup, with a certain recovery target.
 # Choose $until_lsn later than the transaction commit that causes the row
@@ -115,31 +115,44 @@ test_recovery_standby('name', 'standby_4', $node_master, \@recovery_params,
 test_recovery_standby('LSN', 'standby_5', $node_master, \@recovery_params,
 	"5000", $lsn5);
 
-# Multiple targets
-# Last entry has priority (note that an array respects the order of items
-# not hashes).
+# multiple recovery targets are disallowed
+sub test_recovery_multiple_targets
+{
+	my $test_name       = shift;
+	my $node_name       = shift;
+	my $node_master     = shift;
+	my $recovery_params = shift;
+
+	my $node_standby = get_new_node($node_name);
+	$node_standby->init_from_backup($node_master, 'my_backup',
+		has_restoring => 1);
+
+	foreach my $param_item (@$recovery_params)
+	{
+		$node_standby->append_conf('postgresql.conf', qq($param_item));
+	}
+
+	$node_standby->command_fails(['pg_ctl', '-D', $node_standby->data_dir, '-l',
+		$node_standby->logfile, 'start'], "check standby content for $test_name");
+}
+
+@recovery_params = (
+	"recovery_target = 'immediate'",
+    "recovery_target_xid  = '$recovery_txid'",);
+test_recovery_multiple_targets('multi XID', 'standby_6', $node_master, \@recovery_params);
 @recovery_params = (
-	"recovery_target_name = '$recovery_name'",
-	"recovery_target_xid  = '$recovery_txid'",
-	"recovery_target_time = '$recovery_time'");
-test_recovery_standby('name + XID + time',
-	'standby_6', $node_master, \@recovery_params, "3000", $lsn3);
+	"recovery_target = 'immediate'",
+    "recovery_target_time = '$recovery_time'",);
+test_recovery_multiple_targets('multi time', 'standby_7', $node_master, \@recovery_params);
 @recovery_params = (
-	"recovery_target_time = '$recovery_time'",
-	"recovery_target_name = '$recovery_name'",
-	"recovery_target_xid  = '$recovery_txid'");
-test_recovery_standby('time + name + XID',
-	'standby_7', $node_master, \@recovery_params, "2000", $lsn2);
+	"recovery_target = 'immediate'",
+    "recovery_target_name = '$recovery_name'",);
+test_recovery_multiple_targets('multi name', 'standby_8', $node_master, \@recovery_params);
 @recovery_params = (
-	"recovery_target_xid  = '$recovery_txid'",
-	"recovery_target_time = '$recovery_time'",
-	"recovery_target_name = '$recovery_name'");
-test_recovery_standby('XID + time + name',
-	'standby_8', $node_master, \@recovery_params, "4000", $lsn4);
+	"recovery_target = 'immediate'",
+    "recovery_target_lsn = '$recovery_lsn'",);
+test_recovery_multiple_targets('multi LSN', 'standby_9', $node_master, \@recovery_params);
 @recovery_params = (
-	"recovery_target_xid  = '$recovery_txid'",
-	"recovery_target_time = '$recovery_time'",
-	"recovery_target_name = '$recovery_name'",
-	"recovery_target_lsn = '$recovery_lsn'",);
-test_recovery_standby('XID + time + name + LSN',
-	'standby_9', $node_master, \@recovery_params, "5000", $lsn5);
+    "recovery_target_xid  = '$recovery_txid'",
+    "recovery_target = 'immediate'",);
+test_recovery_multiple_targets('multi immediate', 'standby_10', $node_master, \@recovery_params);

Reply via email to