Robert Haas wrote: > > -1. I think it's very useful to have routines for this sort of thing > > that return an error message rather than emitting an error report > > directly. That gives the caller a lot more control.
Michael Paquier wrote: > 1) Consistency with the error messages makes less work for translators, > who already have a lot to deal with. Agreed that the messages can be slightly inconsistent. I tried to make the new messages match the styles of other messages in their respective utilities. Maybe the bigger issue here is inconsistent output styles across the utilities in general: pg_standby.c includes flag names %s: -r maxretries %s pg_basebackup.c writes the settings out in words invalid compression level: %s Note that the final %s in those examples will expand to a more detailed message. For example passing "-Z 10" to pg_dump in the current patch will output: pg_dump: error: invalid compression level: 10 is outside range 0..9 > 2) A centralized error message can provide the same level of details. Even assuming we standardize the message format, different callers have different means to handle the messages. The front-end utilities affected in my patch use calls as varied as fprintf, pg_log_error, write_stderr and pg_fatal. Thus pg_strtoint64_range needs more flexibility than calling pg_log_error internally. > 3) I think that we should not expose directly the status values of > pg_strtoint_status in pg_strtoint64_range(), that's less for module > authors to worry about, and that would be the same approach as we are > using for the wrappers of pg_strto[u]intXX() in the patch of the other > thread (see pg_strto[u]intXX_check for example in [1]). The pg_strto[u]intXX_check functions can return the integer directly only because they handle errors with ereport(ERROR, ...). However, as I mentioned earlier, this is not always what the front-end utilities need to do. -- Joe Nelson https://begriffs.com