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