Re: Improvements in psql hooks for variables

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Daniel Verite <daniel(at)manitou-mail(dot)org>
Cc: Rahila Syed <rahilasyed90(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Improvements in psql hooks for variables
Date: 2016-11-19 23:36:03
Message-ID: 20161119233603.GC13284@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Daniel,

* Daniel Verite (daniel(at)manitou-mail(dot)org) wrote:
> I'm attaching v3 of the patch with error reporting for
> FETCH_COUNT as mentioned upthread, and rebased
> on the most recent master.

Just fyi, there seems to be some issues with this patch because setting
my PROMPT1 and PROMPT2 variables result in rather odd behavior.

Here's what I use:

-------------
\set PROMPT1 '\033[33;1m%M(from '`hostname`').%/.%n.%> [%`date`]\033[0m\n=%x%# '
\set PROMPT2 '-%x%# '
-------------

In reviewing this patch, I also noticed that it's set up to assume a
'true' result when a variable can't be parsed by ParseVariableBool.

-----------
postgres=# \timing off
Timing is off.
postgres=# \timing asdsa
unrecognized value "asdsa" for "\timing": boolean expected
Timing is on.
-----------

That certainly doesn't feel right. I'm thinking that if we're going to
throw an error back to the user about a value being invalid then we
shouldn't change the current value.

My initial thought was that perhaps we should pass the current value to
ParseVariableBool() and let it return whatever the "right" answer is,
however, your use of ParseVariableBool() for enums that happen to accept
on/off means that won't really work.

Perhaps the right answer is to flip this around a bit and treat booleans
as a special case of enums and have a generic solution for enums.
Consider something like:

ParseVariableEnum(valid_enums, str_value, name, curr_value);

'valid_enums' would be a simple list of what the valid values are for a
given variable and their corresponding value, str_value the string the
user typed, name the name of the variable, and curr_value the current
value of the variable.

ParseVariableEnum() could then detect if the string passed in is valid
or not and report to the user if it's incorrect and leave the existing
value alone. This could also generically handle the question of if the
string passed in is a unique prefix of a correct value by comparing it
to all of the valid values and seeing if there's a unique match or not.

Thoughts?

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-11-19 23:44:33 Re: Improvements in psql hooks for variables
Previous Message John Gorman 2016-11-19 22:54:01 Re: Hash tables in dynamic shared memory