Improve validation of recovery_target_xid GUC values.

Previously, the recovery_target_xid GUC values were not sufficiently validated.
As a result, clearly invalid inputs such as the string "bogus", a decimal value
like "1.1", or 0 (a transaction ID smaller than the minimum valid value of 3)
were unexpectedly accepted. In these cases, the value was interpreted as
transaction ID 0, which could cause recovery to behave unexpectedly.

This commit improves validation of recovery_target_xid GUC so that invalid
values are rejected with an error. This prevents recovery from proceeding
with misconfigured recovery_target_xid settings.

Also this commit updates the documentation to clarify the allowed values
for recovery_target_xid GUC.

Author: David Steele <david@pgbackrest.org>
Reviewed-by: Hüseyin Demir <huseyin.d3r@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/f14463ab-990b-4ae9-a177-998d2677aae0@pgbackrest.org

doc/src/sgml/config.sgml

@@ -4334,6 +4334,21 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
         The precise stopping point is also influenced by
         <xref linkend="guc-recovery-target-inclusive"/>.
        </para>
+
+       <para>
+        The value can be specified as either a 32-bit transaction ID or a 64-bit
+        transaction ID (consisting of an epoch and a 32-bit ID), such as the
+        value returned by <function>pg_current_xact_id()</function>. When a
+        64-bit transaction ID is provided, only its 32-bit transaction ID
+        portion is used as the recovery target. For example, the values
+        4294968296 (epoch 1) and 8589935592 (epoch 2) both refer to the same
+        32-bit transaction ID, 1000.
+       </para>
+
+       <para>
+        The effective transaction ID (the 32-bit portion) must be greater than
+        or equal to 3.
+       </para>
       </listitem>
      </varlistentry>
 

src/backend/access/transam/xlogrecovery.c

@@ -5044,11 +5044,38 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source)
 	{
 		TransactionId xid;
 		TransactionId *myextra;
+		char	   *endp;
+		char	   *val;
 
 		errno = 0;
-		xid = (TransactionId) strtou64(*newval, NULL, 0);
-		if (errno == EINVAL || errno == ERANGE)
+
+		/*
+		 * Consume leading whitespace to determine if number is negative
+		 */
+		val = *newval;
+
+		while (isspace((unsigned char) *val))
+			val++;
+
+		/*
+		 * This cast will remove the epoch, if any
+		 */
+		xid = (TransactionId) strtou64(val, &endp, 0);
+
+		if (*endp != '\0' || errno == EINVAL || errno == ERANGE || *val == '-')
+		{
+			GUC_check_errdetail("\"%s\" is not a valid number.",
+								"recovery_target_xid");
+			return false;
+		}
+
+		if (xid < FirstNormalTransactionId)
+		{
+			GUC_check_errdetail("\"%s\" without epoch must be greater than or equal to %u.",
+								"recovery_target_xid",
+								FirstNormalTransactionId);
 			return false;
+		}
 
 		myextra = (TransactionId *) guc_malloc(LOG, sizeof(TransactionId));
 		if (!myextra)

src/test/recovery/t/003_recovery_targets.pl

@@ -240,4 +240,26 @@ ok(!$res, 'invalid timeline target (upper bound check)');
 $log_start =
   $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
 
+# Invalid recovery_target_xid tests
+my ($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_xid TO 'bogus'");
+like(
+	$stderr,
+	qr/is not a valid number/,
+	"invalid recovery_target_xid (bogus value)");
+
+($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_xid TO '-1'");
+like(
+	$stderr,
+	qr/is not a valid number/,
+	"invalid recovery_target_xid (negative)");
+
+($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_xid TO '0'");
+like(
+	$stderr,
+	qr/without epoch must be greater than or equal to 3/,
+	"invalid recovery_target_xid (lower bound check)");
+
 done_testing();