Re: Surprising behaviour of \set AUTOCOMMIT ON
| От | Rahila Syed |
|---|---|
| Тема | Re: Surprising behaviour of \set AUTOCOMMIT ON |
| Дата | |
| Msg-id | CAH2L28ukew2CikwzsMxeAm_x2NSAZROopr-PTfen-TiPdBb+AQ@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Surprising behaviour of \set AUTOCOMMIT ON ("Daniel Verite" <daniel@manitou-mail.org>) |
| Ответы |
Re: Surprising behaviour of \set AUTOCOMMIT ON
|
| Список | pgsql-hackers |
Thank you for comments.
>But there's a function in startup.c which might be the ideal location
>or the check, as it looks like the one and only place where the
>autocommit flag actually changes:
>static void
>autocommit_hook(const char *newval)
Thank you for pointing out this. This indeed is the only place where autocommit setting changes in psql. However, >But there's a function in startup.c which might be the ideal location
>or the check, as it looks like the one and only place where the
>autocommit flag actually changes:
>static void
>autocommit_hook(const char *newval)
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. This will change the existing definition of the hook
which returns void. This will also lead to changes in other implementations of this hook apart from autocommit_hook.
>There are other values than ON: true/yes and theirs abbreviations,
>the value 1, and anything that doesn't resolve to OFF is taken as ON.
>ParseVariableBool() in commands.c already does the job of converting
>these to bool, the new code should probably just call that function
>instead of parsing the value itself.
I have included this in the attached patch.
Also I have included prior review comments by Rushabh Lathia and Amit Langote in the attached patch
Regarding suggestion to move the code inside SetVariable(), I think it is not a good idea because
SetVariable() and variable.c file in which it is present is about handling of psql variables, checks for
correct variable names, values etc. This check is about correctness of the command so it is better placed
in exec_command().
Thank you,
Rahila Syed
On Sat, Sep 3, 2016 at 4:39 PM, Daniel Verite <daniel@manitou-mail.org> wrote:
Rushabh Lathia wrote:
> It might happen that SetVariable() can be called from other places in
> future,
> so its good to have all the set variable related checks inside
> SetVariable().
There's already another way to skip the \set check:
select 'on' as "AUTOCOMMIT" \gset
But there's a function in startup.c which might be the ideal location
for the check, as it looks like the one and only place where the
autocommit flag actually changes:
static void
autocommit_hook(const char *newval)
{
pset.autocommit = ParseVariableBool(newval, "AUTOCOMMIT");}
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
Вложения
В списке pgsql-hackers по дате отправления: