Re: Improvements in psql hooks for variables

You may want to add this to the November commitfest
https://commitfest.postgresql.org/11/.

On Fri, Sep 16, 2016 at 6:05 PM, Daniel Verite <daniel@manitou-mail.org> wrote:
>  Hi,
>
> Following the discussion on forbidding an AUTOCOMMIT off->on
> switch mid-transaction [1], attached is a patch that let the hooks
> return a boolean indicating whether a change is allowed.
>
> Using the hooks, bogus assignments to built-in variables can
> be dealt with more strictly.
>
> For example, pre-patch behavior:
>
>   =# \set ECHO errors
>   =# \set ECHO on
>   unrecognized value "on" for "ECHO"; assuming "none"
>   =# \echo :ECHO
>   on
>
> which has two problems:
> - we have to assume a value, even though we can't know what the user meant.
> - after assignment, the user-visible value of the variable diverges from its
> internal counterpart (pset.echo in this case).
>
>
> Post-patch:
>   =# \set ECHO errors
>   =# \set ECHO on
>   unrecognized value "on" for "ECHO"
>   \set: error while setting variable
>   =# \echo :ECHO
>   errors
>
> Both the internal pset.* state and the user-visible value are kept unchanged
> is the input value is incorrect.
>
> Concerning AUTOCOMMIT, autocommit_hook() could return false to forbid
> a switch when the conditions are not met.
>
>
> Another user-visible effect of the patch is that, using a bogus value
> for a built-in variable on the command-line becomes a fatal error
> that prevents psql to continue.
> This is not directly intended by the patch but is a consequence
> of SetVariable() failing.
>
> Example:
>   $ ./psql -vECHO=bogus
>   unrecognized value "bogus" for "ECHO"
>   psql: could not set variable "ECHO"
>   $ echo $?
>   1
>
> The built-in vars concerned by the change are:
>
> booleans: AUTOCOMMIT, ON_ERROR_STOP, QUIET, SINGLELINE, SINGLESTEP
>
> non-booleans: ECHO, ECHO_HIDDEN, ON_ERROR_ROLLBACK, COMP_KEYWORD_CASE,
>  HISTCONTROL, VERBOSITY, SHOW_CONTEXT
>
> We could go further to close the gap between pset.* and the built-in
> variables,
> by changing how they're initialized and forbidding deletion as Tom
> suggests in [2], but if there's negative feedback on the above changes,
> I think we should hear it first.
>
> [1]
> https://www.postgresql.org/message-id/f2cb5838-0ee9-4fe3-acc0-df77aeb7d4c7%40mm
> [2]
> https://www.postgresql.org/message-id/4695.1473961140%40sss.pgh.pa.us
>
>
> Best regards,
> --
> Daniel Vérité
> PostgreSQL-powered mailer: http://www.manitou-mail.org
> Twitter: @DanielVerite
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>



--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company