Rahila Syed <rahilasyed90@gmail.com> writes:
>> Looking at the other variables hooks, they already emit errors and can
>> deny the effect of a change corresponding to a new value, without
>> informing the caller. Why would autocommit be different?
> These instances where psql_error occurs inside hooks the command is
> successful and the value supplied by user is reinterpreted to some other
> value as user had supplied an unrecognisable value.
> With psql_error_on_autocommit patch what was intended was to make
> the command unsuccessful and keep the previous setting of autocommit.
> Hence having it inside autocommit_hook did not seem appropriate to me.
Nonetheless, asking all callers of SetVariable to deal with such cases
is entirely unmaintainable/unacceptable. Have you considered expanding
the API for hook functions? I'm not really sure why we didn't provide a
way for the hooks to reject a setting to begin with.
Actually, it would make a lot more sense UI-wise if attempting to assign a
non-boolean value to a boolean variable resulted in an error and no change
to the variable, instead of what happens now.
Anyway, I'm not very thrilled with the idea that AUTOCOMMIT is so special
that it should have a different behavior than any other built-in psql
variable. If we make them all throw errors and refuse to change to bad
values, that would be consistent and defensible IMO. But having
AUTOCOMMIT alone act that way is not a feature, it's a wart.
regards, tom lane