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,
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