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
signature.asc
Description: PGP signature
