public inbox for [email protected]  

help / color / mirror / Atom feed

Improve checks for GUC recovery_target_xid
20+ messages / 4 participants
[nested] [flat]

* Improve checks for GUC recovery_target_xid
@ 2026-02-20 05:41 David Steele <[email protected]>
  2026-02-26 07:20 ` Re: Improve checks for GUC recovery_target_xid Hüseyin Demir <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  0 siblings, 2 replies; 20+ messages in thread

From: David Steele @ 2026-02-20 05:41 UTC (permalink / raw)
  To: pgsql-hackers

Hackers,

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Also add a comment that truncation of the input value is expected since
users will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).

This patch was promised in [1] -- here at last!

Regards,
-David

[1] 
https://www.postgresql.org/message-id/cf04b7c6-7774-4ffb-86f5-ca85462d5fd6%40pgbackrest.org
From 0f3c34a5f9503535b046ea5821ae07a9e48c185c Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Fri, 20 Feb 2026 05:40:28 +0000
Subject: Improve checks for GUC recovery_target_xid

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Also add a comment that truncation of the input value is expected since
users will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).
---
 src/backend/access/transam/xlogrecovery.c   | 22 +++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 34 +++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index c0c2744d45b..554a8fcfe59 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -5107,11 +5107,29 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source)
 	{
 		TransactionId xid;
 		TransactionId *myextra;
+		char          *endp;
 
 		errno = 0;
-		xid = (TransactionId) strtou64(*newval, NULL, 0);
-		if (errno == EINVAL || errno == ERANGE)
+
+		/*
+		 * This cast will remove the epoch, if any
+		 */
+		xid = (TransactionId) strtou64(*newval, &endp, 0);
+
+		if (*endp != '\0' || errno == EINVAL || errno == ERANGE)
+		{
+			GUC_check_errdetail("\"%s\" is not a valid number.",
+								"recovery_target_xid");
+			return false;
+		}
+
+		if (xid < FirstNormalTransactionId)
+		{
+			GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",
+								"recovery_target_xid",
+								FirstNormalTransactionId);
 			return false;
+		}
 
 		myextra = (TransactionId *) guc_malloc(LOG, sizeof(TransactionId));
 		if (!myextra)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..1ff3f3c8676 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -240,4 +240,38 @@ ok(!$res, 'invalid timeline target (upper bound check)');
 $log_start =
   $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
 
+# Invalid xid target
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_xid = 'bogus'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid xid target (bogus value)');
+
+$log_start = $node_standby->wait_for_log("is not a valid number");
+
+# Timeline target out of min range
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_xid = '0'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid xid target (lower bound check)');
+
+$log_start = $node_standby->wait_for_log(
+	"without epoch must greater than or equal to 3", $log_start);
+
 done_testing();
-- 
2.34.1



Attachments:

  [text/plain] recovery-target-xid-v1.patch (3.8K, 2-recovery-target-xid-v1.patch)
  download | inline diff:
From 0f3c34a5f9503535b046ea5821ae07a9e48c185c Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Fri, 20 Feb 2026 05:40:28 +0000
Subject: Improve checks for GUC recovery_target_xid

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Also add a comment that truncation of the input value is expected since
users will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).
---
 src/backend/access/transam/xlogrecovery.c   | 22 +++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 34 +++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index c0c2744d45b..554a8fcfe59 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -5107,11 +5107,29 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source)
 	{
 		TransactionId xid;
 		TransactionId *myextra;
+		char          *endp;
 
 		errno = 0;
-		xid = (TransactionId) strtou64(*newval, NULL, 0);
-		if (errno == EINVAL || errno == ERANGE)
+
+		/*
+		 * This cast will remove the epoch, if any
+		 */
+		xid = (TransactionId) strtou64(*newval, &endp, 0);
+
+		if (*endp != '\0' || errno == EINVAL || errno == ERANGE)
+		{
+			GUC_check_errdetail("\"%s\" is not a valid number.",
+								"recovery_target_xid");
+			return false;
+		}
+
+		if (xid < FirstNormalTransactionId)
+		{
+			GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",
+								"recovery_target_xid",
+								FirstNormalTransactionId);
 			return false;
+		}
 
 		myextra = (TransactionId *) guc_malloc(LOG, sizeof(TransactionId));
 		if (!myextra)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..1ff3f3c8676 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -240,4 +240,38 @@ ok(!$res, 'invalid timeline target (upper bound check)');
 $log_start =
   $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
 
+# Invalid xid target
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_xid = 'bogus'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid xid target (bogus value)');
+
+$log_start = $node_standby->wait_for_log("is not a valid number");
+
+# Timeline target out of min range
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_xid = '0'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid xid target (lower bound check)');
+
+$log_start = $node_standby->wait_for_log(
+	"without epoch must greater than or equal to 3", $log_start);
+
 done_testing();
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
@ 2026-02-26 07:20 ` Hüseyin Demir <[email protected]>
  2026-03-04 05:07   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  1 sibling, 1 reply; 20+ messages in thread

From: Hüseyin Demir @ 2026-02-26 07:20 UTC (permalink / raw)
  To: [email protected]; +Cc: David Steele <[email protected]>

Hi David, 

The approach LGTM and checked the previous patch too. I have a few things to add. 

The following grammar can be changed by adding "without epoch must be greater than or equal to %u"
+           GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",
+                               "recovery_target_xid",
+                               FirstNormalTransactionId);


Secondly, 

The comment on the lower-bound XID test says # Timeline target out of min range — should be # XID target out of min range.

+# Timeline target out of min range
+$node_standby->append_conf('postgresql.conf',
+   "recovery_target_xid = '0'");
+

When it comes to *endp validations I suppose the validation passes when we provide   recovery_target_xid = '-1'.  This passes the endp validation and FirstNormalTransactionId checks. Is it a valid approach to allow negative values to this GUC ? 

When -1 is provided the following checks allow them to be a valid GUC. 

+       /*
+        * This cast will remove the epoch, if any
+        */
+       xid = (TransactionId) strtou64(*newval, &endp, 0);
+
+       if (*endp != '\0' || errno == EINVAL || errno == ERANGE)
+       {
+           GUC_check_errdetail("\"%s\" is not a valid number.",
+                               "recovery_target_xid");
+           return false;
+       }


Regards.

^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-26 07:20 ` Re: Improve checks for GUC recovery_target_xid Hüseyin Demir <[email protected]>
@ 2026-03-04 05:07   ` David Steele <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: David Steele @ 2026-03-04 05:07 UTC (permalink / raw)
  To: Hüseyin Demir <[email protected]>; [email protected]; Fujii Masao <[email protected]>

Thanks for having a look at this!

On 2/26/26 14:20, Hüseyin Demir wrote:
> 
> The following grammar can be changed by adding "without epoch must be greater than or equal to %u"
> +           GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",
> +                               "recovery_target_xid",
> +                               FirstNormalTransactionId);

Oops - fixed!

> The comment on the lower-bound XID test says # Timeline target out of min range — should be # XID target out of min range.

I have fixed this and made the comments more consistent overall.

> When it comes to *endp validations I suppose the validation passes when we provide   recovery_target_xid = '-1'.  This passes the endp validation and FirstNormalTransactionId checks. Is it a valid approach to allow negative values to this GUC ?
> 
> When -1 is provided the following checks allow them to be a valid GUC.

Yeah, -1 should not be allowed here. I've updated the code to error on 
negative numbers but probably we should import strtou64_strict from the 
front end code or use strtou32_strict, though that needs to be discussed 
separately.

Thanks,
-David
From 488260c30dd65aae2f0b9077641dbe7e195ea8f4 Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Wed, 4 Mar 2026 04:47:23 +0000
Subject: Improve checks for GUC recovery_target_xid

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Also add a comment that truncation of the input value is expected since
users will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).
---
 src/backend/access/transam/xlogrecovery.c   | 31 +++++++++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 34 +++++++++++++++++++++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ecd66fd86a4..fd0345a65f8 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -5108,11 +5108,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)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..0813394d70a 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -240,4 +240,38 @@ 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 (bogus value)
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_xid = 'bogus'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid recovery_target_xid (bogus value)');
+
+$log_start = $node_standby->wait_for_log("is not a valid number");
+
+# Invalid recovery_target_xid (lower bound check)
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_xid = '0'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid recovery_target_xid (lower bound check)');
+
+$log_start = $node_standby->wait_for_log(
+	"without epoch must be greater than or equal to 3", $log_start);
+
 done_testing();
-- 
2.34.1



Attachments:

  [text/plain] recovery-target-xid-v2.patch (4.1K, 2-recovery-target-xid-v2.patch)
  download | inline diff:
From 488260c30dd65aae2f0b9077641dbe7e195ea8f4 Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Wed, 4 Mar 2026 04:47:23 +0000
Subject: Improve checks for GUC recovery_target_xid

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Also add a comment that truncation of the input value is expected since
users will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).
---
 src/backend/access/transam/xlogrecovery.c   | 31 +++++++++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 34 +++++++++++++++++++++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ecd66fd86a4..fd0345a65f8 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -5108,11 +5108,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)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..0813394d70a 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -240,4 +240,38 @@ 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 (bogus value)
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_xid = 'bogus'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid recovery_target_xid (bogus value)');
+
+$log_start = $node_standby->wait_for_log("is not a valid number");
+
+# Invalid recovery_target_xid (lower bound check)
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_xid = '0'");
+
+$res = run_log(
+	[
+		'pg_ctl',
+		'--pgdata' => $node_standby->data_dir,
+		'--log' => $node_standby->logfile,
+		'start',
+	]);
+ok(!$res, 'invalid recovery_target_xid (lower bound check)');
+
+$log_start = $node_standby->wait_for_log(
+	"without epoch must be greater than or equal to 3", $log_start);
+
 done_testing();
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
@ 2026-02-27 01:12 ` Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  1 sibling, 1 reply; 20+ messages in thread

From: Fujii Masao @ 2026-02-27 01:12 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: pgsql-hackers

On Fri, Feb 20, 2026 at 2:42 PM David Steele <[email protected]> wrote:
>
> Hackers,
>
> Currently check_recovery_target_xid() converts invalid values to 0. So,
> for example, the following configuration added to postgresql.conf
> followed by a startup:
>
> recovery_target_xid = 'bogus'
> recovery_target_xid = '1.1'
> recovery_target_xid = '0'
>
> ... does not generate an error but recovery does not complete. There are
> many values that can prevent recovery from completing but we should at
> least catch obvious misconfiguration by the user.

+1


> The origin of the problem is that we do not perform a range check in the
> GUC value passed-in for recovery_target_xid. This commit improves the
> situation by using adding end checking to strtou64() and by providing
> stricter range checks. Some test cases are added for the cases of an
> incorrect or a lower-bound timeline value, checking the sanity of the
> reports based on the contents of the server logs.
>
> Also add a comment that truncation of the input value is expected since
> users will generally be using the output from pg_current_xact_id() (or
> similar) to set recovery_target_xid (just as our tests do).
>
> This patch was promised in [1] -- here at last!

Thanks for the patch!

+ GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",

"must greater" shiould be "must be greater"?

"without epoch" seems not necessary to me.

+ /*
+ * This cast will remove the epoch, if any
+ */
+ xid = (TransactionId) strtou64(*newval, &endp, 0);

Would it be better to use strtouint32_strict() instead of strtou64()?
That would allow us to detect invalid XID values larger than 2^32 and
report an error, similar to what pg_resetwal -x does.

Regards,

-- 
Fujii Masao






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
@ 2026-03-04 05:11   ` David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: David Steele @ 2026-03-04 05:11 UTC (permalink / raw)
  To: Fujii Masao <[email protected]>; Hüseyin Demir <[email protected]>; +Cc: pgsql-hackers

On 2/27/26 08:12, Fujii Masao wrote:
> On Fri, Feb 20, 2026 at 2:42 PM David Steele <[email protected]> wrote:
> 
> + GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",
> 
> "must greater" shiould be "must be greater"?

Fixed in v2 attached to the prior email.

> "without epoch" seems not necessary to me.

I guess that depends on whether or not we error if the epoch is present, 
see below.

> + /*
> + * This cast will remove the epoch, if any
> + */
> + xid = (TransactionId) strtou64(*newval, &endp, 0);
> 
> Would it be better to use strtouint32_strict() instead of strtou64()?
> That would allow us to detect invalid XID values larger than 2^32 and
> report an error, similar to what pg_resetwal -x does.

This was my first instinct, but it causes our integration tests to fail 
because pg_current_xact_id() returns the xid with epoch. You can fix 
this by casting pg_current_xact_id()::xid but this seems like a pretty 
big change in usage. I'm OK with it but we'd definitely need to update 
the documentation to match.

What do you think?

Regards
-David





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
@ 2026-03-04 15:41     ` Fujii Masao <[email protected]>
  2026-03-04 16:19       ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  0 siblings, 2 replies; 20+ messages in thread

From: Fujii Masao @ 2026-03-04 15:41 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: Hüseyin Demir <[email protected]>; pgsql-hackers

On Wed, Mar 4, 2026 at 2:11 PM David Steele <[email protected]> wrote:
>
> On 2/27/26 08:12, Fujii Masao wrote:
> > On Fri, Feb 20, 2026 at 2:42 PM David Steele <[email protected]> wrote:
> >
> > + GUC_check_errdetail("\"%s\" without epoch must greater than or equal to %u.",
> >
> > "must greater" shiould be "must be greater"?
>
> Fixed in v2 attached to the prior email.
>
> > "without epoch" seems not necessary to me.
>
> I guess that depends on whether or not we error if the epoch is present,
> see below.
>
> > + /*
> > + * This cast will remove the epoch, if any
> > + */
> > + xid = (TransactionId) strtou64(*newval, &endp, 0);
> >
> > Would it be better to use strtouint32_strict() instead of strtou64()?
> > That would allow us to detect invalid XID values larger than 2^32 and
> > report an error, similar to what pg_resetwal -x does.
>
> This was my first instinct, but it causes our integration tests to fail
> because pg_current_xact_id() returns the xid with epoch. You can fix
> this by casting pg_current_xact_id()::xid but this seems like a pretty
> big change in usage. I'm OK with it but we'd definitely need to update
> the documentation to match.
>
> What do you think?

I see your point, and I'm fine with the patch.

My earlier comment was based on my misunderstanding. I thought that only
32-bit transaction IDs were allowed for recovery_target_xid, and therefore
values larger than 2^32 should be rejected.

However, recovery_target_xid currently accepts both 32-bit XIDs and 64-bit
full transaction IDs (epoch + 32-bit XID). When a 64-bit value is specified,
only the 32-bit XID portion is used as the recovery target.
Given this behavior, your change seems to make sense.

Regarding the regression test, if the purpose is to verify the GUC hook
for recovery_target_xid, it might be simpler to test whether
"ALTER SYSTEM SET recovery_target_xid TO ..." succeeds or fails as expected
as follows, rather than starting the server with that setting. That said,
since recovery_target_timeline is already tested in a similar way, I understand
why you followed the same pattern here. So I'm ok with the current approach.

    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)");

If we think it's better to use ALTER SYSTEM SET for testing invalid
recovery_target_xxx settings to keep the regression tests simpler,
we can revisit this later and address it in a separate patch.

Regards,

-- 
Fujii Masao





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
@ 2026-03-04 16:19       ` Fujii Masao <[email protected]>
  1 sibling, 0 replies; 20+ messages in thread

From: Fujii Masao @ 2026-03-04 16:19 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: Hüseyin Demir <[email protected]>; pgsql-hackers

On Thu, Mar 5, 2026 at 12:41 AM Fujii Masao <[email protected]> wrote:
> My earlier comment was based on my misunderstanding. I thought that only
> 32-bit transaction IDs were allowed for recovery_target_xid, and therefore
> values larger than 2^32 should be rejected.
>
> However, recovery_target_xid currently accepts both 32-bit XIDs and 64-bit
> full transaction IDs (epoch + 32-bit XID). When a 64-bit value is specified,
> only the 32-bit XID portion is used as the recovery target.
> Given this behavior, your change seems to make sense.

I'm tempted to clarify this behavior by adding something like
the following text to the description of recovery_target_xid
in config.sgml...:

-------------------------------
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 pg_current_xact_id(). 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.

The effective transaction ID (the 32-bit portion) must be greater than
or equal to 3.
-------------------------------

Regards,


-- 
Fujii Masao





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
@ 2026-03-05 03:40       ` David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  1 sibling, 1 reply; 20+ messages in thread

From: David Steele @ 2026-03-05 03:40 UTC (permalink / raw)
  To: Fujii Masao <[email protected]>; +Cc: Hüseyin Demir <[email protected]>; pgsql-hackers

On 3/4/26 22:41, Fujii Masao wrote:
> 
> Regarding the regression test, if the purpose is to verify the GUC hook
> for recovery_target_xid, it might be simpler to test whether
> "ALTER SYSTEM SET recovery_target_xid TO ..." succeeds or fails as expected
> as follows, rather than starting the server with that setting. That said,
> since recovery_target_timeline is already tested in a similar way, I understand
> why you followed the same pattern here. So I'm ok with the current approach.

I wrote the tests for recovery_target_timeline but I was not too 
satisfied with them because starting Postgres is fairly expensive.

> 
>      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)");
> 
> If we think it's better to use ALTER SYSTEM SET for testing invalid
> recovery_target_xxx settings to keep the regression tests simpler,
> we can revisit this later and address it in a separate patch.

I've updated the patch to do it this way. Not only is it faster but you 
get a better message when the expected value is incorrect.

I can update the tests for recovery_target_timeline in a separate patch.

Regards,
-David

From b2be59a52a7a74cd0a6c83619fa782a92b7ab9a0 Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Thu, 5 Mar 2026 03:35:32 +0000
Subject: Improve checks for GUC recovery_target_xid

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Add a comment that truncation of the input value is expected since users
will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).

Also update the documentation for recovery_target_xid to clarify usage.
---
 doc/src/sgml/config.sgml                    | 15 ++++++++++
 src/backend/access/transam/xlogrecovery.c   | 31 +++++++++++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 27 ++++++++++++++++++
 3 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f670e2d4c31..949b86e4e70 100644
--- a/doc/src/sgml/config.sgml
+++ b/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>
 
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ecd66fd86a4..fd0345a65f8 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -5108,11 +5108,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)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..f5da13128d6 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -240,4 +240,31 @@ 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
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->start;
+
+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();
-- 
2.34.1



Attachments:

  [text/plain] recovery-target-xid-v3.patch (5.2K, 2-recovery-target-xid-v3.patch)
  download | inline diff:
From b2be59a52a7a74cd0a6c83619fa782a92b7ab9a0 Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Thu, 5 Mar 2026 03:35:32 +0000
Subject: Improve checks for GUC recovery_target_xid

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Add a comment that truncation of the input value is expected since users
will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).

Also update the documentation for recovery_target_xid to clarify usage.
---
 doc/src/sgml/config.sgml                    | 15 ++++++++++
 src/backend/access/transam/xlogrecovery.c   | 31 +++++++++++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 27 ++++++++++++++++++
 3 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f670e2d4c31..949b86e4e70 100644
--- a/doc/src/sgml/config.sgml
+++ b/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>
 
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ecd66fd86a4..fd0345a65f8 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -5108,11 +5108,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)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..f5da13128d6 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -240,4 +240,31 @@ 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
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+	has_restoring => 1);
+$node_standby->start;
+
+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();
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
@ 2026-03-05 04:03         ` Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Michael Paquier @ 2026-03-05 04:03 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: Fujii Masao <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On Thu, Mar 05, 2026 at 03:40:44AM +0000, David Steele wrote:
> I wrote the tests for recovery_target_timeline but I was not too satisfied
> with them because starting Postgres is fairly expensive.

+# Invalid recovery_target_xid tests
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup($node_primary, 'my_backup',
+    has_restoring => 1);
+$node_standby->start;
+
+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)");

Smart move to rely on ALTER SYSTEM to check how the GUC callback is
reacting on incorrect input values.  Why do you need to create and
start a new standby if it is not used, then?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, 2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
@ 2026-03-05 04:21           ` David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: David Steele @ 2026-03-05 04:21 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Fujii Masao <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On 3/5/26 11:03, Michael Paquier wrote:
> On Thu, Mar 05, 2026 at 03:40:44AM +0000, David Steele wrote:
>> I wrote the tests for recovery_target_timeline but I was not too satisfied
>> with them because starting Postgres is fairly expensive.
> 
> +# Invalid recovery_target_xid tests
> +$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
> +$node_standby->init_from_backup($node_primary, 'my_backup',
> +    has_restoring => 1);
> +$node_standby->start;
> +
> +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)");
> 
> Smart move to rely on ALTER SYSTEM to check how the GUC callback is
> reacting on incorrect input values.  Why do you need to create and
> start a new standby if it is not used, then?

The prior standby is not running because of the invalid config. I 
figured it was better to start clean but when I update the 
recovery_target_timeline tests I was planning to use the same standby 
for all the new tests.

Regards,
-David





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
@ 2026-03-05 05:03             ` Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Fujii Masao @ 2026-03-05 05:03 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: Michael Paquier <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On Thu, Mar 5, 2026 at 1:21 PM David Steele <[email protected]> wrote:
> The prior standby is not running because of the invalid config. I
> figured it was better to start clean but when I update the
> recovery_target_timeline tests I was planning to use the same standby
> for all the new tests.

Alternatively, we can use $node_primary, since ALTER SYSTEM SET with
invalid recovery_target_timeline or recovery_target_xid does not
affect the primary.

Regards,

-- 
Fujii Masao





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
@ 2026-03-05 08:15               ` David Steele <[email protected]>
  2026-03-05 12:42                 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: David Steele @ 2026-03-05 08:15 UTC (permalink / raw)
  To: Fujii Masao <[email protected]>; +Cc: Michael Paquier <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On 3/5/26 12:03, Fujii Masao wrote:
> On Thu, Mar 5, 2026 at 1:21 PM David Steele <[email protected]> wrote:
>> The prior standby is not running because of the invalid config. I
>> figured it was better to start clean but when I update the
>> recovery_target_timeline tests I was planning to use the same standby
>> for all the new tests.
> 
> Alternatively, we can use $node_primary, since ALTER SYSTEM SET with
> invalid recovery_target_timeline or recovery_target_xid does not
> affect the primary.

Well, as it turns out I was using the primary after all because I copied 
your example and forgot to update the host. Seems weird to set these 
GUCs on the primary but as long as we get the expected errors I don't 
suppose it matters.

Regards,
-David
From 77fdf7b365d9887ea52522248ba2d5d575825f4b Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Thu, 5 Mar 2026 08:14:36 +0000
Subject: Improve checks for GUC recovery_target_xid

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Add a comment that truncation of the input value is expected since users
will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).

Also update the documentation for recovery_target_xid to clarify usage.
---
 doc/src/sgml/config.sgml                    | 15 ++++++++++
 src/backend/access/transam/xlogrecovery.c   | 31 +++++++++++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 22 +++++++++++++++
 3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f670e2d4c31..949b86e4e70 100644
--- a/doc/src/sgml/config.sgml
+++ b/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>
 
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ecd66fd86a4..fd0345a65f8 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -5108,11 +5108,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)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..4e36e3a3fb5 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/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();
-- 
2.34.1



Attachments:

  [text/plain] recovery-target-xid-v4.patch (5.0K, 2-recovery-target-xid-v4.patch)
  download | inline diff:
From 77fdf7b365d9887ea52522248ba2d5d575825f4b Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Thu, 5 Mar 2026 08:14:36 +0000
Subject: Improve checks for GUC recovery_target_xid

Currently check_recovery_target_xid() converts invalid values to 0. So,
for example, the following configuration added to postgresql.conf
followed by a startup:

recovery_target_xid = 'bogus'
recovery_target_xid = '1.1'
recovery_target_xid = '0'

... does not generate an error but recovery does not complete. There are
many values that can prevent recovery from completing but we should at
least catch obvious misconfiguration by the user.

The origin of the problem is that we do not perform a range check in the
GUC value passed-in for recovery_target_xid. This commit improves the
situation by using adding end checking to strtou64() and by providing
stricter range checks. Some test cases are added for the cases of an
incorrect or a lower-bound timeline value, checking the sanity of the
reports based on the contents of the server logs.

Add a comment that truncation of the input value is expected since users
will generally be using the output from pg_current_xact_id() (or
similar) to set recovery_target_xid (just as our tests do).

Also update the documentation for recovery_target_xid to clarify usage.
---
 doc/src/sgml/config.sgml                    | 15 ++++++++++
 src/backend/access/transam/xlogrecovery.c   | 31 +++++++++++++++++++--
 src/test/recovery/t/003_recovery_targets.pl | 22 +++++++++++++++
 3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f670e2d4c31..949b86e4e70 100644
--- a/doc/src/sgml/config.sgml
+++ b/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>
 
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ecd66fd86a4..fd0345a65f8 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -5108,11 +5108,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)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..4e36e3a3fb5 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/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();
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
@ 2026-03-05 12:42                 ` Fujii Masao <[email protected]>
  2026-03-05 13:32                   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Fujii Masao @ 2026-03-05 12:42 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: Michael Paquier <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On Thu, Mar 5, 2026 at 5:15 PM David Steele <[email protected]> wrote:
>
> On 3/5/26 12:03, Fujii Masao wrote:
> > On Thu, Mar 5, 2026 at 1:21 PM David Steele <[email protected]> wrote:
> >> The prior standby is not running because of the invalid config. I
> >> figured it was better to start clean but when I update the
> >> recovery_target_timeline tests I was planning to use the same standby
> >> for all the new tests.
> >
> > Alternatively, we can use $node_primary, since ALTER SYSTEM SET with
> > invalid recovery_target_timeline or recovery_target_xid does not
> > affect the primary.
>
> Well, as it turns out I was using the primary after all because I copied
> your example and forgot to update the host. Seems weird to set these
> GUCs on the primary but as long as we get the expected errors I don't
> suppose it matters.

Thanks for updating the patch! I've pushed the patch.

Regards,

-- 
Fujii Masao





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 12:42                 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
@ 2026-03-05 13:32                   ` David Steele <[email protected]>
  2026-03-05 15:04                     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: David Steele @ 2026-03-05 13:32 UTC (permalink / raw)
  To: Fujii Masao <[email protected]>; +Cc: Michael Paquier <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On 3/5/26 19:42, Fujii Masao wrote:
> On Thu, Mar 5, 2026 at 5:15 PM David Steele <[email protected]> wrote:
>>
>> On 3/5/26 12:03, Fujii Masao wrote:
>>> On Thu, Mar 5, 2026 at 1:21 PM David Steele <[email protected]> wrote:
>>>> The prior standby is not running because of the invalid config. I
>>>> figured it was better to start clean but when I update the
>>>> recovery_target_timeline tests I was planning to use the same standby
>>>> for all the new tests.
>>>
>>> Alternatively, we can use $node_primary, since ALTER SYSTEM SET with
>>> invalid recovery_target_timeline or recovery_target_xid does not
>>> affect the primary.
>>
>> Well, as it turns out I was using the primary after all because I copied
>> your example and forgot to update the host. Seems weird to set these
>> GUCs on the primary but as long as we get the expected errors I don't
>> suppose it matters.
> 
> Thanks for updating the patch! I've pushed the patch.

Excellent, thank you!

Attached are the test changes for recovery_target_timeline. I can start 
a new thread and add it to the next CF if you like, but since it is just 
test changes maybe we can fast track it.

Regards,
-David
From c70d06d415852e1d7b9bd44d17e9efaf491dfc1d Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Thu, 5 Mar 2026 13:27:58 +0000
Subject: Improve tests for recovery_target_timeline GUC.

Update the recovery_target_timeline tests to match the simpler format
used for recovery_target_xid in bffd7130.
---
 src/test/recovery/t/003_recovery_targets.pl | 68 ++++++---------------
 1 file changed, 20 insertions(+), 48 deletions(-)

diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 4e36e3a3fb5..047eb13293a 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -190,58 +190,30 @@ like(
 	qr/FATAL: .* recovery ended before configured recovery target was reached/,
 	'recovery end before target reached is a fatal error');
 
-# Invalid timeline target
-$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
-$node_standby->init_from_backup($node_primary, 'my_backup',
-	has_restoring => 1);
-$node_standby->append_conf('postgresql.conf',
-	"recovery_target_timeline = 'bogus'");
-
-$res = run_log(
-	[
-		'pg_ctl',
-		'--pgdata' => $node_standby->data_dir,
-		'--log' => $node_standby->logfile,
-		'start',
-	]);
-ok(!$res, 'invalid timeline target (bogus value)');
-
-my $log_start = $node_standby->wait_for_log("is not a valid number");
-
-# Timeline target out of min range
-$node_standby->append_conf('postgresql.conf',
-	"recovery_target_timeline = '0'");
-
-$res = run_log(
-	[
-		'pg_ctl',
-		'--pgdata' => $node_standby->data_dir,
-		'--log' => $node_standby->logfile,
-		'start',
-	]);
-ok(!$res, 'invalid timeline target (lower bound check)');
-
-$log_start =
-  $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
-
-# Timeline target out of max range
-$node_standby->append_conf('postgresql.conf',
-	"recovery_target_timeline = '4294967296'");
+# Invalid recovery_target_timeline tests
+my ($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_timeline TO 'bogus'");
+like(
+	$stderr,
+	qr/is not a valid number/,
+	"invalid recovery_target_timeline (bogus value)");
 
-$res = run_log(
-	[
-		'pg_ctl',
-		'--pgdata' => $node_standby->data_dir,
-		'--log' => $node_standby->logfile,
-		'start',
-	]);
-ok(!$res, 'invalid timeline target (upper bound check)');
+($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_timeline TO '0'");
+like(
+	$stderr,
+	qr/must be between 1 and 4294967295/,
+	"invalid recovery_target_timeline (lower bound check)");
 
-$log_start =
-  $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
+($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_timeline TO '4294967296'");
+like(
+	$stderr,
+	qr/must be between 1 and 4294967295/,
+	"invalid recovery_target_timeline (upper bound check)");
 
 # Invalid recovery_target_xid tests
-my ($result, $stdout, $stderr) = $node_primary->psql('postgres',
+($result, $stdout, $stderr) = $node_primary->psql('postgres',
 	"ALTER SYSTEM SET recovery_target_xid TO 'bogus'");
 like(
 	$stderr,
-- 
2.34.1



Attachments:

  [text/plain] recovery-target-timeline-v1.patch (3.1K, 2-recovery-target-timeline-v1.patch)
  download | inline diff:
From c70d06d415852e1d7b9bd44d17e9efaf491dfc1d Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Thu, 5 Mar 2026 13:27:58 +0000
Subject: Improve tests for recovery_target_timeline GUC.

Update the recovery_target_timeline tests to match the simpler format
used for recovery_target_xid in bffd7130.
---
 src/test/recovery/t/003_recovery_targets.pl | 68 ++++++---------------
 1 file changed, 20 insertions(+), 48 deletions(-)

diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 4e36e3a3fb5..047eb13293a 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -190,58 +190,30 @@ like(
 	qr/FATAL: .* recovery ended before configured recovery target was reached/,
 	'recovery end before target reached is a fatal error');
 
-# Invalid timeline target
-$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
-$node_standby->init_from_backup($node_primary, 'my_backup',
-	has_restoring => 1);
-$node_standby->append_conf('postgresql.conf',
-	"recovery_target_timeline = 'bogus'");
-
-$res = run_log(
-	[
-		'pg_ctl',
-		'--pgdata' => $node_standby->data_dir,
-		'--log' => $node_standby->logfile,
-		'start',
-	]);
-ok(!$res, 'invalid timeline target (bogus value)');
-
-my $log_start = $node_standby->wait_for_log("is not a valid number");
-
-# Timeline target out of min range
-$node_standby->append_conf('postgresql.conf',
-	"recovery_target_timeline = '0'");
-
-$res = run_log(
-	[
-		'pg_ctl',
-		'--pgdata' => $node_standby->data_dir,
-		'--log' => $node_standby->logfile,
-		'start',
-	]);
-ok(!$res, 'invalid timeline target (lower bound check)');
-
-$log_start =
-  $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
-
-# Timeline target out of max range
-$node_standby->append_conf('postgresql.conf',
-	"recovery_target_timeline = '4294967296'");
+# Invalid recovery_target_timeline tests
+my ($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_timeline TO 'bogus'");
+like(
+	$stderr,
+	qr/is not a valid number/,
+	"invalid recovery_target_timeline (bogus value)");
 
-$res = run_log(
-	[
-		'pg_ctl',
-		'--pgdata' => $node_standby->data_dir,
-		'--log' => $node_standby->logfile,
-		'start',
-	]);
-ok(!$res, 'invalid timeline target (upper bound check)');
+($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_timeline TO '0'");
+like(
+	$stderr,
+	qr/must be between 1 and 4294967295/,
+	"invalid recovery_target_timeline (lower bound check)");
 
-$log_start =
-  $node_standby->wait_for_log("must be between 1 and 4294967295", $log_start);
+($result, $stdout, $stderr) = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_timeline TO '4294967296'");
+like(
+	$stderr,
+	qr/must be between 1 and 4294967295/,
+	"invalid recovery_target_timeline (upper bound check)");
 
 # Invalid recovery_target_xid tests
-my ($result, $stdout, $stderr) = $node_primary->psql('postgres',
+($result, $stdout, $stderr) = $node_primary->psql('postgres',
 	"ALTER SYSTEM SET recovery_target_xid TO 'bogus'");
 like(
 	$stderr,
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 12:42                 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 13:32                   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
@ 2026-03-05 15:04                     ` Fujii Masao <[email protected]>
  2026-03-06 06:15                       ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Fujii Masao @ 2026-03-05 15:04 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: Michael Paquier <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On Thu, Mar 5, 2026 at 10:32 PM David Steele <[email protected]> wrote:
>
> On 3/5/26 19:42, Fujii Masao wrote:
> > On Thu, Mar 5, 2026 at 5:15 PM David Steele <[email protected]> wrote:
> >>
> >> On 3/5/26 12:03, Fujii Masao wrote:
> >>> On Thu, Mar 5, 2026 at 1:21 PM David Steele <[email protected]> wrote:
> >>>> The prior standby is not running because of the invalid config. I
> >>>> figured it was better to start clean but when I update the
> >>>> recovery_target_timeline tests I was planning to use the same standby
> >>>> for all the new tests.
> >>>
> >>> Alternatively, we can use $node_primary, since ALTER SYSTEM SET with
> >>> invalid recovery_target_timeline or recovery_target_xid does not
> >>> affect the primary.
> >>
> >> Well, as it turns out I was using the primary after all because I copied
> >> your example and forgot to update the host. Seems weird to set these
> >> GUCs on the primary but as long as we get the expected errors I don't
> >> suppose it matters.
> >
> > Thanks for updating the patch! I've pushed the patch.
>
> Excellent, thank you!
>
> Attached are the test changes for recovery_target_timeline. I can start
> a new thread and add it to the next CF if you like, but since it is just
> test changes maybe we can fast track it.

Yes, let's discuss and review the patch in this thread.

Thanks for the patch! It looks good to me. Barring any objections, I
will commit it.

Regards,

-- 
Fujii Masao





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 12:42                 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 13:32                   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 15:04                     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
@ 2026-03-06 06:15                       ` Michael Paquier <[email protected]>
  2026-03-06 07:05                         ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Michael Paquier @ 2026-03-06 06:15 UTC (permalink / raw)
  To: Fujii Masao <[email protected]>; +Cc: David Steele <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On Fri, Mar 06, 2026 at 12:04:00AM +0900, Fujii Masao wrote:
> Thanks for the patch! It looks good to me. Barring any objections, I
> will commit it.

Thanks.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, 2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 12:42                 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 13:32                   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 15:04                     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-06 06:15                       ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
@ 2026-03-06 07:05                         ` Fujii Masao <[email protected]>
  2026-03-16 07:44                           ` Re: Improve checks for GUC recovery_target_xid Hüseyin Demir <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Fujii Masao @ 2026-03-06 07:05 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: David Steele <[email protected]>; Hüseyin Demir <[email protected]>; pgsql-hackers

On Fri, Mar 6, 2026 at 3:15 PM Michael Paquier <[email protected]> wrote:
>
> On Fri, Mar 06, 2026 at 12:04:00AM +0900, Fujii Masao wrote:
> > Thanks for the patch! It looks good to me. Barring any objections, I
> > will commit it.
>
> Thanks.

I've pushed the patch. Thanks!

Regards,

-- 
Fujii Masao





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 12:42                 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 13:32                   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 15:04                     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-06 06:15                       ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-06 07:05                         ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
@ 2026-03-16 07:44                           ` Hüseyin Demir <[email protected]>
  2026-03-17 14:36                             ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Hüseyin Demir @ 2026-03-16 07:44 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: Fujii Masao <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers

Hi,


David Steele <[email protected]>, 6 Mar 2026 Cum, 16:01 tarihinde şunu yazdı:
>
> On 3/6/26 14:05, Fujii Masao wrote:
> > On Fri, Mar 6, 2026 at 3:15 PM Michael Paquier <[email protected]> wrote:
> >>
> >> On Fri, Mar 06, 2026 at 12:04:00AM +0900, Fujii Masao wrote:
> >>> Thanks for the patch! It looks good to me. Barring any objections, I
> >>> will commit it.
> >>
> >> Thanks.
> >
> > I've pushed the patch. Thanks!
>
> Thank you and great idea on ALTER SYSTEM. I've been hesitant to add more
> tests in this area because they are so expensive but now I feel much
> better about it. But that's the last for this CF since there is more
> important stuff to be done.
>
> Regards,
> -David

I tried to create tests with the ALTER SYSTEM approach to validate the GUC.

You can review it if it's the correct approach or not. We can create a
new CF record if required for the patch.

Regards.


Attachments:

  [application/octet-stream] v1-0001-add-regression-tests-for-recovery-target-xid-validation.patch (2.1K, 2-v1-0001-add-regression-tests-for-recovery-target-xid-validation.patch)
  download | inline diff:
From a8ff58a7564b4db74385d0130530c45dcc6fd5c7 Mon Sep 17 00:00:00 2001
From: Huseyin Demir <[email protected]>
Date: Mon, 16 Mar 2026 08:27:22 +0100
Subject: [PATCH] Add regression tests for recovery_target_xid GUC validation

Add two test cases to src/test/recovery/t/003_recovery_targets.pl that
verify the check_recovery_target_xid GUC hook correctly rejects invalid
values via ALTER SYSTEM:

  - Non-numeric value ('bogus') -> EINVAL from strtou64
  - Overflow value ('99999999999999999999') -> ERANGE from strtou64

Both tests verify the psql return code and the error message pattern.
Using ALTER SYSTEM on the already-running primary node exercises the same
check hook code path as the conf-edit + pg_ctl-start-failure approach
but is simpler and faster (no standby node creation needed).
---
 src/test/recovery/t/003_recovery_targets.pl | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index e0df1a23423..aba0da150dd 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -240,4 +240,22 @@ 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 via ALTER SYSTEM (non-numeric)
+my $stderr = '';
+$ret = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_xid = 'bogus'",
+	stderr => \$stderr);
+ok($ret != 0, 'ALTER SYSTEM rejects non-numeric recovery_target_xid');
+like($stderr, qr/invalid value for parameter "recovery_target_xid"/,
+	'error message for non-numeric XID value');
+
+# Invalid recovery_target_xid via ALTER SYSTEM (overflow)
+$stderr = '';
+$ret = $node_primary->psql('postgres',
+	"ALTER SYSTEM SET recovery_target_xid = '99999999999999999999'",
+	stderr => \$stderr);
+ok($ret != 0, 'ALTER SYSTEM rejects overflow recovery_target_xid');
+like($stderr, qr/invalid value for parameter "recovery_target_xid"/,
+	'error message for overflow XID value');
+
 done_testing();
-- 
2.50.1 (Apple Git-155)



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 12:42                 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 13:32                   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 15:04                     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-06 06:15                       ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-06 07:05                         ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-16 07:44                           ` Re: Improve checks for GUC recovery_target_xid Hüseyin Demir <[email protected]>
@ 2026-03-17 14:36                             ` David Steele <[email protected]>
  2026-03-17 15:18                               ` Re: Improve checks for GUC recovery_target_xid Hüseyin Demir <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: David Steele @ 2026-03-17 14:36 UTC (permalink / raw)
  To: Hüseyin Demir <[email protected]>; +Cc: Fujii Masao <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers

Hi,

On 3/16/26 14:44, Hüseyin Demir wrote:
> 
> David Steele <[email protected]>, 6 Mar 2026 Cum, 16:01 tarihinde şunu yazdı:
>>
>> On 3/6/26 14:05, Fujii Masao wrote:
>>> On Fri, Mar 6, 2026 at 3:15 PM Michael Paquier <[email protected]> wrote:
>>>>
>>>> On Fri, Mar 06, 2026 at 12:04:00AM +0900, Fujii Masao wrote:
>>>>> Thanks for the patch! It looks good to me. Barring any objections, I
>>>>> will commit it.
>>>>
>>>> Thanks.
>>>
>>> I've pushed the patch. Thanks!
>>
>> Thank you and great idea on ALTER SYSTEM. I've been hesitant to add more
>> tests in this area because they are so expensive but now I feel much
>> better about it. But that's the last for this CF since there is more
>> important stuff to be done.
>>
>> Regards,
>> -David
> 
> I tried to create tests with the ALTER SYSTEM approach to validate the GUC.
> 
> You can review it if it's the correct approach or not. We can create a
> new CF record if required for the patch.

I modified the tests in the patch to use ALTER SYSTEM and that was 
committed at [1].

One of the tests (bogus) you have added here is a duplicate but the 
other one (upper bound) could be added.

You appear to be working against an old version of the master branch so 
I would recommend rebasing and then add your upper bound test following 
the test pattern we have already established.

I personally don't think the upper bound test adds a lot of value here 
since it is handled by strtou64() just like the bogus test so it will 
not extend coverage, but I'm fine with it if others are.

Regards,
-David

[1] 
https://git.postgresql.org/pg/commitdiff/bffd7130e942e2bd45153ab09e5fab70e74ece58.





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Improve checks for GUC recovery_target_xid
  2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-02-27 01:12 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-04 05:11   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-04 15:41     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 03:40       ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 04:03         ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-05 04:21           ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 05:03             ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 08:15               ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 12:42                 ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-05 13:32                   ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
  2026-03-05 15:04                     ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-06 06:15                       ` Re: Improve checks for GUC recovery_target_xid Michael Paquier <[email protected]>
  2026-03-06 07:05                         ` Re: Improve checks for GUC recovery_target_xid Fujii Masao <[email protected]>
  2026-03-16 07:44                           ` Re: Improve checks for GUC recovery_target_xid Hüseyin Demir <[email protected]>
  2026-03-17 14:36                             ` Re: Improve checks for GUC recovery_target_xid David Steele <[email protected]>
@ 2026-03-17 15:18                               ` Hüseyin Demir <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Hüseyin Demir @ 2026-03-17 15:18 UTC (permalink / raw)
  To: David Steele <[email protected]>; +Cc: Fujii Masao <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers

Thanks for the information, I think it's already covered by the previous
commit.

Regards.



David Steele <[email protected]>, 17 Mar 2026 Sal, 15:36 tarihinde şunu
yazdı:

> Hi,
>
> On 3/16/26 14:44, Hüseyin Demir wrote:
> >
> > David Steele <[email protected]>, 6 Mar 2026 Cum, 16:01 tarihinde
> şunu yazdı:
> >>
> >> On 3/6/26 14:05, Fujii Masao wrote:
> >>> On Fri, Mar 6, 2026 at 3:15 PM Michael Paquier <[email protected]>
> wrote:
> >>>>
> >>>> On Fri, Mar 06, 2026 at 12:04:00AM +0900, Fujii Masao wrote:
> >>>>> Thanks for the patch! It looks good to me. Barring any objections, I
> >>>>> will commit it.
> >>>>
> >>>> Thanks.
> >>>
> >>> I've pushed the patch. Thanks!
> >>
> >> Thank you and great idea on ALTER SYSTEM. I've been hesitant to add more
> >> tests in this area because they are so expensive but now I feel much
> >> better about it. But that's the last for this CF since there is more
> >> important stuff to be done.
> >>
> >> Regards,
> >> -David
> >
> > I tried to create tests with the ALTER SYSTEM approach to validate the
> GUC.
> >
> > You can review it if it's the correct approach or not. We can create a
> > new CF record if required for the patch.
>
> I modified the tests in the patch to use ALTER SYSTEM and that was
> committed at [1].
>
> One of the tests (bogus) you have added here is a duplicate but the
> other one (upper bound) could be added.
>
> You appear to be working against an old version of the master branch so
> I would recommend rebasing and then add your upper bound test following
> the test pattern we have already established.
>
> I personally don't think the upper bound test adds a lot of value here
> since it is handled by strtou64() just like the bogus test so it will
> not extend coverage, but I'm fine with it if others are.
>
> Regards,
> -David
>
> [1]
>
> https://git.postgresql.org/pg/commitdiff/bffd7130e942e2bd45153ab09e5fab70e74ece58
> .
>


^ permalink  raw  reply  [nested|flat] 20+ messages in thread

end of thread, other threads:[~2026-03-17 15:18 UTC | newest]

Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-20 05:41 Improve checks for GUC recovery_target_xid David Steele <[email protected]>
2026-02-26 07:20 ` Hüseyin Demir <[email protected]>
2026-03-04 05:07   ` David Steele <[email protected]>
2026-02-27 01:12 ` Fujii Masao <[email protected]>
2026-03-04 05:11   ` David Steele <[email protected]>
2026-03-04 15:41     ` Fujii Masao <[email protected]>
2026-03-04 16:19       ` Fujii Masao <[email protected]>
2026-03-05 03:40       ` David Steele <[email protected]>
2026-03-05 04:03         ` Michael Paquier <[email protected]>
2026-03-05 04:21           ` David Steele <[email protected]>
2026-03-05 05:03             ` Fujii Masao <[email protected]>
2026-03-05 08:15               ` David Steele <[email protected]>
2026-03-05 12:42                 ` Fujii Masao <[email protected]>
2026-03-05 13:32                   ` David Steele <[email protected]>
2026-03-05 15:04                     ` Fujii Masao <[email protected]>
2026-03-06 06:15                       ` Michael Paquier <[email protected]>
2026-03-06 07:05                         ` Fujii Masao <[email protected]>
2026-03-16 07:44                           ` Hüseyin Demir <[email protected]>
2026-03-17 14:36                             ` David Steele <[email protected]>
2026-03-17 15:18                               ` Hüseyin Demir <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox