Re: Surprising behaviour of \set AUTOCOMMIT ON

Hello,

>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?
The only type of psql_error inside hooks is as follows,

psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
newval, "ECHO", "none");

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.

But as pointed out by you, the other way of setting autocommit i,e

*SELECT 'on' as "AUTOCOMMIT" \gset* will not be handled by the patch.
So I will change the patch to have the check in the autocommit_hook instead.

This will mean if *\set AUTOCOMMIT ON* or *SELECT 'on' as "AUTOCOMMIT"
\gset *
is run inside a transaction, it will be effective after current transaction
has
ended. Appropriate message will be displayed notifying this to the user and
user need not
rerun the set AUTOCOMMIT command.

Thank you,
Rahila Syed

On Thu, Sep 8, 2016 at 9:55 PM, Daniel Verite <daniel(at)manitou-mail(dot)org>
wrote:

> Rahila Syed wrote:
>
> > However, including the check here will require returning the status
> > of command from this hook. i.e if we throw error inside this
> > hook we will need to return false indicating the value has not changed.
>
> 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?
>
> For example echo_hook does this:
>
> /* ...if the value is in (queries,errors,all,none) then
> assign pset.echo accordingly ... */
> else
> {
> psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n",
> newval, "ECHO", "none");
> pset.echo = PSQL_ECHO_NONE;
> }
>
>
> If the user issues \set ECHO FOOBAR, then it produces the above error
> message and makes the same internal change as if \set ECHO none
> had been issued.
>
> But, the value of the variable as seen by the user is still FOOBAR:
>
> \set
> [...other variables...]
> ECHO = 'FOOBAR'
>
> The proposed patch doesn't follow that behavior, as it denies
> a new value for AUTOCOMMIT. You might argue that it's better,
> but on the other side, there are two reasons against it:
>
> - it's not consistent with the other uses of \set that affect psql,
> such as ECHO, ECHO_HIDDEN, ON_ERROR_ROLLBACK,
> COMP_KEYWORD_CASE... and even AUTOCOMMIT as
> \set AUTOCOMMIT FOOBAR is not denied, just reinterpreted.
>
> - it doesn't address the case of another method than \set being used
> to set the variable, as in : SELECT 'on' as "AUTOCOMMIT" \gset
> whereas the hook would work in that case.
>
> Best regards,
> --
> Daniel Vérité
> PostgreSQL-powered mailer: http://www.manitou-mail.org
> Twitter: @DanielVerite
>