Re: Surprising behaviour of \set AUTOCOMMIT ON - Mailing list pgsql-hackers
| From | Rahila Syed |
|---|---|
| Subject | Re: Surprising behaviour of \set AUTOCOMMIT ON |
| Date | |
| Msg-id | CAH2L28vU9tR8cLmuR3khy0QeHi2PxgAK6UpbmKHQugROETHuNA@mail.gmail.com Whole thread Raw |
| In response to | Re: Surprising behaviour of \set AUTOCOMMIT ON ("Daniel Verite" <daniel@manitou-mail.org>) |
| Responses |
Re: Surprising behaviour of \set AUTOCOMMIT ON
|
| List | pgsql-hackers |
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
>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");
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@manitou-mail.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
pgsql-hackers by date: