Re: Improve checks for GUC recovery_target_xid

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.