On Wed, May 15, 2019 at 03:19:29AM +0900, Fujii Masao wrote:
> + if (strncasecmp(opt_str, "true", 4) != 0 &&
> + strncasecmp(opt_str, "false", 5) != 0)
>
> Shouldn't we allow also "on" and "off", "1", "0" as a valid boolean value,
> like VACUUM does?
I am wondering, in order to keep this patch simple, if you shouldn't
accept any value and just let the parsing logic on the backend side
do all the work. That's what we do for other things like the
connection parameter replication for example, and there is no need to
mimic a boolean parsing equivalent on the frontend with something like
check_bool_str() as presented in the patch. The main downside is that
the error message gets linked to VACUUM and not vacuumdb.
Another thing which you may be worth looking at would be to make
parse_bool() frontend aware, where pg_strncasecmp() is actually
available.
--
Michael
