Re: Improvements in psql hooks for variables

Tom Lane wrote:

> Moreover, the committed patch is inconsistent in that it forbids
> only one of the above. Why is it okay to treat unset as "off",
> but not okay to treat the default empty-string value as "on"?

Treating unset (NULL in the value) as "off" comes from the fact
that except AUTOCOMMIT, our built-in variables are not initialized
with a default value. For instance we call this in EstablishVariableSpace();
SetVariableAssignHook(pset.vars, "ON_ERROR_STOP", on_error_stop_hook);
then on_error_stop_hook is called with NULL as the value
then ParseVariableBool(NULL, "ON_ERROR_STOP", &pset.on_error_stop)
is what initializes pset.on_error_stop to false.

The same happens if/when the variable is unset. Again the hook is called
with NULL, and it sets back the pset.* variable in a hardcoded default state,
which is false for all booleans.

Incidentally I want to suggest to change that, so that all variables
should be initialized with a non-NULL value right from the start, and the
value would possibly come to NULL only if it's unset.
This would allow the hook to distinguish between initialization and
unsetting, which in turn will allow it to deny the \unset in the
cases when it doesn't make any sense conceptually (like AUTOCOMMIT).

Forcing values for our built-in variables would also avoid the following:

=# \echo :ON_ERROR_STOP
:ON_ERROR_STOP

Even if the variable ON_ERROR_STOP does exist in the VariableSpace
and has a hook and there's an initialized corresponding pset.on_error_stop,
from the user's viewpoint, it's as if the variable doesn't exist
until they intialize it explicitly.
I suggest that it doesn't make much sense and it would be better
to have that instead right from the start:

=# \echo :ON_ERROR_STOP
off

> One possible compromise that would address your concern about display
> is to modify the hook API some more so that variable hooks could actually
> substitute new values. Then for example the bool-variable hooks could
> effectively replace "\set AUTOCOMMIT" by "\set AUTOCOMMIT on" and
> "\unset AUTOCOMMIT" by "\set AUTOCOMMIT off", ensuring that interrogation
> of their values always produces sane results.

Agreed, that looks like a good compromise.

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite