Re: [GENERAL] psql \pset pager

Lists: pgsql-generalpgsql-hackerspgsql-patches
From: Steve Crawford <scrawford(at)pinpointresearch(dot)com>
To: pgsql-general(at)postgresql(dot)org
Subject: psql \pset pager
Date: 2008-04-29 20:34:19
Message-ID: 4817864B.2000705@pinpointresearch.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers pgsql-patches

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


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Steve Crawford <scrawford(at)pinpointresearch(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org>
Subject: Re: [GENERAL] psql \pset pager
Date: 2008-05-06 15:37:57
Message-ID: 200805061537.m46FbvJ07619@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers pgsql-patches

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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Steve Crawford <scrawford(at)pinpointresearch(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [GENERAL] psql \pset pager
Date: 2008-05-06 15:56:42
Message-ID: 14862.1210089402@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers pgsql-patches

Bruce Momjian <bruce(at)momjian(dot)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


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Steve Crawford <scrawford(at)pinpointresearch(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [GENERAL] psql \pset pager
Date: 2008-05-06 21:34:49
Message-ID: 200805062134.m46LYng10880@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers pgsql-patches

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Steve Crawford <scrawford(at)pinpointresearch(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [GENERAL] psql \pset pager
Date: 2008-05-06 22:19:20
Message-ID: 25608.1210112360@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers pgsql-patches

Bruce Momjian <bruce(at)momjian(dot)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


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Steve Crawford <scrawford(at)pinpointresearch(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [HACKERS] [GENERAL] psql \pset pager
Date: 2008-05-07 02:34:17
Message-ID: 200805070234.m472YHV27262@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers pgsql-patches

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachment Content-Type Size
/rtmp/diff text/x-diff 2.2 KB

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Steve Crawford <scrawford(at)pinpointresearch(dot)com>
Cc: pgsql-general(at)postgresql(dot)org
Subject: Re: psql \pset pager
Date: 2008-05-15 00:22:15
Message-ID: 200805150022.m4F0MF202412@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers pgsql-patches

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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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


From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: "Bruce Momjian" <bruce(at)momjian(dot)us>
Cc: "Steve Crawford" <scrawford(at)pinpointresearch(dot)com>, <pgsql-general(at)postgresql(dot)org>
Subject: Re: psql \pset pager
Date: 2008-05-15 01:11:51
Message-ID: 87skwk5q20.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers pgsql-patches

"Bruce Momjian" <bruce(at)momjian(dot)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!