Re: Improvements in psql hooks for variables

Daniel Verite <daniel@manitou-mail.org>

From: "Daniel Verite" <daniel@manitou-mail.org>
To: "Stephen Frost" <sfrost@snowman.net>
Cc: "Tom Lane" <tgl@sss.pgh.pa.us>,"Rahila Syed" <rahilasyed90@gmail.com>,"Ashutosh Bapat" <ashutosh.bapat@enterprisedb.com>,"pgsql-hackers" <pgsql-hackers@postgresql.org>
Date: 2016-11-21T16:37:36Z
Lists: pgsql-hackers
	Stephen Frost wrote:

> Are you working to make those changes?  I'd rather we make the changes
> to this code once rather than push what you have now only to turn around
> and change it significantly again.

So I went through the psql commands which don't fail on parse errors
for booleans. I'd like to attract attention on \c, because I see
several options.

\c [-reuse-previous=BOOL] ...other args..

Current: if we can't parse BOOL, assume it's ON, and go on with reconnecting.

Option1: if we can't parse BOOL, stop here, don't reconnect, set
the command result as "failed", also ignoring the other arguments.

Option2: maybe we want to create a difference between interactive
and non-interactive use, as there's already one in handling
the failure to connect through \c.
The manpage says:
   If the connection attempt failed (wrong user name, access denied,
   etc.), the previous connection will only be kept if psql is in
   interactive mode. When executing a non-interactive script,
   processing will immediately stop with an error.

Proposal: if interactive, same as Option1.
If not interactive, -reuse-previous=bogus has the same outcome
as a failure to connect. Which I think means two subcases:
if it's through \i then kill the connection but don't quit psql
if it's through -f then quit psql.

Option3: leave this command unchanged, avoiding trouble.

\timing BOOL

Current: non-parseable BOOL interpreted as TRUE. Empty BOOL toggles the
state.

Proposal: on non-parseable BOOL, command failure and pset.timing is 
left unchanged.

\pset [x | expanded | vertical ] BOOL
\pset numericlocale BOOL
\pset [tuples_only | t] BOOL
\pset footer BOOL
\t BOOL (falls into pset_do("tuples_only", ...))
\pset pager BOOL

Current: non-parseable non-empty BOOL interpreted as TRUE. Empty BOOL
toggles the on/off state. In some cases, BOOL interpretation is attempted
only after specific built-in values have been ruled out first.

Proposal: non-parseable BOOL implies command failure and unchanged state.

About the empty string when a BOOL is expected. Only \c -reuse-previous
seems concerned:

#= \c -reuse-previous=
acts the same as
#= \c -reuse-previous=ON

Proposal: handle empty as when the value is bogus.

The other commands interpret this lack of value specifically to toggle
the state, so it's a non-issue for them.

Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


Commits

  1. Clean up psql's behavior for a few more control variables.

  2. Make psql's \set display variables in alphabetical order.

  3. Improve psql's behavior for \set and \unset of its control variables.

  4. Make psql reject attempts to set special variables to invalid values.