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
passesthe 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.
