Re: Improvements in psql hooks for variables

"Daniel Verite" <daniel(at)manitou-mail(dot)org> writes:
> Tom Lane wrote:
>> Also, why aren't you using ParseVariableBool's existing ability to report
>> errors?

> Its' because there are two cases:
> - either only a boolean can be given to the command or variable,
> in which we let ParseVariableBool() tell:
> unrecognized value "bogus" for "command": boolean expected
> - or other parameters besides boolean are acceptable, in which case we
> can't say "boolean expected", because in fact a boolean is no more or
> less expected than the other valid values.

Ah. Maybe it's time for a ParseVariableEnum, or some other way of
dealing with those cases in a more unified fashion.

> Do we care about the "boolean expected" part of the error message?

I'm not especially in love with that particular wording, but I'm doubtful
that we should give up all indication of what valid values are, especially
in the cases where there are more than just bool-equivalent values.
I think the right thing to do here is to fix it so that the input routine
has full information about all the valid values. On the backend side,
we've gone to considerable lengths to make sure that error messages are
helpful for such cases, eg:

regression=# set backslash_quote to foo;
ERROR: invalid value for parameter "backslash_quote": "foo"
HINT: Available values: safe_encoding, on, off.

and I think it may be worth working equally hard here.

> I get the idea, except that in this example, the compiler is
> unhappy because popt->topt.expanded is not a bool, and an
> explicit cast here would be kludgy.

Yeah, you'll need an intermediate variable if you're trying to use
ParseVariableBool for such a case.

regards, tom lane