Re: Surprising behaviour of \set AUTOCOMMIT ON

Поиск
Список
Период
Сортировка
От Rahila Syed
Тема Re: Surprising behaviour of \set AUTOCOMMIT ON
Дата
Msg-id CAH2L28vU9tR8cLmuR3khy0QeHi2PxgAK6UpbmKHQugROETHuNA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Surprising behaviour of \set AUTOCOMMIT ON  ("Daniel Verite" <daniel@manitou-mail.org>)
Ответы Re: Surprising behaviour of \set AUTOCOMMIT ON  (Tom Lane <tgl@sss.pgh.pa.us>)
Список 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
>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@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 по дате отправления:

Предыдущее
От: Matteo Beccati
Дата:
Сообщение: Re: kqueue
Следующее
От: Anastasia Lubennikova
Дата:
Сообщение: Re: sequences and pg_upgrade