psql \pset pager

Started by Steve Crawfordover 17 years ago8 messages
#1Steve Crawford
scrawford@pinpointresearch.com

My fingers sometimes run on "autoappend semicolon" mode and I end up
typing "\pset pager always;" instead of "\pset pager always". No error
is returned, short (but wide) output is not routed to the pager, and I
have to back up and correct the \pset pager command. After some
experimentation, I found that any unrecognized string sets the pager to
be used for long output:

steve=> \pset pager on;
Pager is used for long output.

steve=> \pset pager off;
Pager is used for long output.

steve=> \pset pager always;
Pager is used for long output.

steve=> \pset pager occasionally
Pager is used for long output.

steve=> \pset pager at random
Pager is used for long output.
\pset: extra argument "random" ignored

The above commands set the pager to be used for long output regardless
of the prior setting. Bad input doesn't generate errors except in the
case where there are too many parameters.

I didn't find this documented. Is the acceptance of bad input by design
or an oversight?

Also, what would be the feasibility of having psql route output to the
pager if the output is too long or too _wide_? I end up with too wide at
least as often as too long.

Cheers,
Steve

#2Bruce Momjian
bruce@momjian.us
In reply to: Steve Crawford (#1)
Re: [GENERAL] psql \pset pager

Steve Crawford wrote:

My fingers sometimes run on "autoappend semicolon" mode and I end up
typing "\pset pager always;" instead of "\pset pager always". No error
is returned, short (but wide) output is not routed to the pager, and I
have to back up and correct the \pset pager command. After some
experimentation, I found that any unrecognized string sets the pager to
be used for long output:

steve=> \pset pager on;
Pager is used for long output.

steve=> \pset pager off;
Pager is used for long output.

steve=> \pset pager always;
Pager is used for long output.

steve=> \pset pager occasionally
Pager is used for long output.

steve=> \pset pager at random
Pager is used for long output.
\pset: extra argument "random" ignored

The above commands set the pager to be used for long output regardless
of the prior setting. Bad input doesn't generate errors except in the
case where there are too many parameters.

I didn't find this documented. Is the acceptance of bad input by design
or an oversight?

Also, what would be the feasibility of having psql route output to the
pager if the output is too long or too _wide_? I end up with too wide at
least as often as too long.

[ moved to hackers list]

I looked at the psql code and found:

bool
ParseVariableBool(const char *val)
{
if (val == NULL)
return false; /* not set -> assume "off" */
if (pg_strcasecmp(val, "off") == 0)
return false; /* accept "off" or "OFF" as true */

/*
* for backwards compatibility, anything except "off" or "OFF" is
* taken as "true"
*/
return true;
}

So, I think the answer is that we have the current behavior because of
backward compatibility. Perhaps we should be more strict in
ParseVariableBool(), perhaps only allowing true/false and on/off.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: [GENERAL] psql \pset pager

Bruce Momjian <bruce@momjian.us> writes:

So, I think the answer is that we have the current behavior because of
backward compatibility. Perhaps we should be more strict in
ParseVariableBool(), perhaps only allowing true/false and on/off.

If we're going to change it, we should make it match GUC's parse_bool,
which has had some actual thought put into it.

regards, tom lane

#4Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#3)
Re: [GENERAL] psql \pset pager

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

So, I think the answer is that we have the current behavior because of
backward compatibility. Perhaps we should be more strict in
ParseVariableBool(), perhaps only allowing true/false and on/off.

If we're going to change it, we should make it match GUC's parse_bool,
which has had some actual thought put into it.

Good idea. Do I copy the C code into /psql or somehow share the
function?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#4)
Re: [GENERAL] psql \pset pager

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

If we're going to change it, we should make it match GUC's parse_bool,
which has had some actual thought put into it.

Good idea. Do I copy the C code into /psql or somehow share the
function?

Just copy it --- it's not large enough to be worth doing something like
inventing a /port module for, and besides it's not clear that you want
exactly the same API.

regards, tom lane

#6Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#5)
1 attachment(s)
Re: [HACKERS] [GENERAL] psql \pset pager

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

If we're going to change it, we should make it match GUC's parse_bool,
which has had some actual thought put into it.

Good idea. Do I copy the C code into /psql or somehow share the
function?

Just copy it --- it's not large enough to be worth doing something like
inventing a /port module for, and besides it's not clear that you want
exactly the same API.

Patch attached and applied.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Attachments:

/rtmp/difftext/x-diffDownload
Index: src/bin/psql/variables.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/variables.c,v
retrieving revision 1.28
diff -c -c -r1.28 variables.c
*** src/bin/psql/variables.c	1 Jan 2008 19:45:56 -0000	1.28
--- src/bin/psql/variables.c	7 May 2008 02:10:55 -0000
***************
*** 48,68 ****
  	return NULL;
  }
  
  bool
! ParseVariableBool(const char *val)
  {
! 	if (val == NULL)
  		return false;			/* not set -> assume "off" */
- 	if (pg_strcasecmp(val, "off") == 0)
- 		return false;			/* accept "off" or "OFF" as true */
  
! 	/*
! 	 * for backwards compatibility, anything except "off" or "OFF" is taken as
! 	 * "true"
! 	 */
  	return true;
  }
  
  /*
   * Read numeric variable, or defaultval if it is not set, or faultval if its
   * value is not a valid numeric string.  If allowtrail is false, this will
--- 48,95 ----
  	return NULL;
  }
  
+ /*
+  * Try to interpret value as boolean value.  Valid values are: true,
+  * false, yes, no, on, off, 1, 0; as well as unique prefixes thereof.
+  */
  bool
! ParseVariableBool(const char *value)
  {
! 	size_t		len;
! 
! 	if (value == NULL)
  		return false;			/* not set -> assume "off" */
  
! 	len = strlen(value);
! 
! 	if (pg_strncasecmp(value, "true", len) == 0)
! 		return true;
! 	else if (pg_strncasecmp(value, "false", len) == 0)
! 		return false;
! 	else if (pg_strncasecmp(value, "yes", len) == 0)
! 		return true;
! 	else if (pg_strncasecmp(value, "no", len) == 0)
! 		return false;
! 	/* 'o' is not unique enough */
! 	else if (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0)
! 		return true;
! 	else if (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0)
! 		return false;
! 	else if (pg_strcasecmp(value, "1") == 0)
! 		return true;
! 	else if (pg_strcasecmp(value, "0") == 0)
! 		return false;
! 	else
! 	{
! 		/* NULL is treated as false, so a non-matching value is 'true' */
! 		psql_error("unrecognized boolean value; assuming \"on\".\n");
! 		return true;
! 	}
! 	/* suppress compiler warning */
  	return true;
  }
  
+ 
  /*
   * Read numeric variable, or defaultval if it is not set, or faultval if its
   * value is not a valid numeric string.  If allowtrail is false, this will
#7Bruce Momjian
bruce@momjian.us
In reply to: Steve Crawford (#1)
Re: psql \pset pager

Steve Crawford wrote:

My fingers sometimes run on "autoappend semicolon" mode and I end up
typing "\pset pager always;" instead of "\pset pager always". No error
is returned, short (but wide) output is not routed to the pager, and I
have to back up and correct the \pset pager command. After some
experimentation, I found that any unrecognized string sets the pager to
be used for long output:

steve=> \pset pager on;
Pager is used for long output.

steve=> \pset pager off;
Pager is used for long output.

steve=> \pset pager always;
Pager is used for long output.

steve=> \pset pager occasionally
Pager is used for long output.

steve=> \pset pager at random
Pager is used for long output.
\pset: extra argument "random" ignored

The above commands set the pager to be used for long output regardless
of the prior setting. Bad input doesn't generate errors except in the
case where there are too many parameters.

I didn't find this documented. Is the acceptance of bad input by design
or an oversight?

Will be fixed in 8.4.

Also, what would be the feasibility of having psql route output to the
pager if the output is too long or too _wide_? I end up with too wide at
least as often as too long.

Also done for 8.4.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#8Gregory Stark
stark@enterprisedb.com
In reply to: Bruce Momjian (#7)
Re: psql \pset pager

"Bruce Momjian" <bruce@momjian.us> writes:

Steve Crawford wrote:

My fingers sometimes run on "autoappend semicolon" mode and I end up
typing "\pset pager always;" instead of "\pset pager always".

FWIW I get bitten by this all the time with \set AUTOCOMMIT.

I didn't find this documented. Is the acceptance of bad input by design
or an oversight?

Will be fixed in 8.4.

I expect there'll be some screams from people with scripts which start
failing. I wonder whether we won't have second thoughts by then.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's Slony Replication support!