Re: pretty print viewdefs

Lists: pgsql-hackers
From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: pretty print viewdefs
Date: 2009-08-26 14:02:33
Message-ID: 4A954079.9060704@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


[originally sent from wrong account :-( ]

The tiny patch attached fixes a long-standing peeve of mine (and at
least one of my clients'), namely that the target list printed in
viewdefs are unreadable.

example output now looks like this:

regression=# select pg_get_viewdef('shoe',true);
pg_get_viewdef
-----------------------------------------------
SELECT
sh.shoename,
sh.sh_avail,
sh.slcolor,
sh.slminlen,
sh.slminlen * un.un_fact AS slminlen_cm,
sh.slmaxlen,
sh.slmaxlen * un.un_fact AS slmaxlen_cm,
sh.slunit
FROM shoe_data sh, unit un
WHERE sh.slunit = un.un_name;

Is there any objection?

cheers

andrew

Attachment Content-Type Size
prettyprint.patch text/x-patch 599 bytes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 14:18:03
Message-ID: 11844.1251296283@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> The tiny patch attached fixes a long-standing peeve of mine (and at
> least one of my clients'), namely that the target list printed in
> viewdefs are unreadable.

Personally I think this will take up enough vertical space to make
things less readable on-screen, not more so. But that's just MHO.
It probably depends a lot on the sorts of views you tend to look at...

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 14:43:42
Message-ID: 4A954A1E.1070601@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> The tiny patch attached fixes a long-standing peeve of mine (and at
>> least one of my clients'), namely that the target list printed in
>> viewdefs are unreadable.
>>
>
> Personally I think this will take up enough vertical space to make
> things less readable on-screen, not more so. But that's just MHO.
> It probably depends a lot on the sorts of views you tend to look at...
>
>
>

Well, I could work out if the bit that will be added to the line will
run it over some limit (like 80 chars) and only put in the line break
then, but it would involve a lot more code.

When you're dealing with a view that has 40 or 50 fields, having them
all run together over a dozen or two lines is just horrible.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 14:55:45
Message-ID: 16079.1251298545@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> When you're dealing with a view that has 40 or 50 fields, having them
> all run together over a dozen or two lines is just horrible.

True, but is having them span a couple of screens vertically going to
be much better? There'll be a whole lot of wasted whitespace.

I'm not dead set against this change, just trying to consider
alternative viewpoints.

regards, tom lane


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 14:55:52
Message-ID: 162867790908260755k1eba4f3dn8d0ef59ea6f23745@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2009/8/26 Andrew Dunstan <andrew(at)dunslane(dot)net>:
>
> [originally sent from wrong account :-( ]
>
>
> The tiny patch attached fixes a long-standing peeve of mine (and at least
> one of my clients'), namely that the target list printed in viewdefs are
> unreadable.
>
> example output now looks like this:
>
>   regression=# select pg_get_viewdef('shoe',true);
>                   pg_get_viewdef
> -----------------------------------------------
>     SELECT
>        sh.shoename,
>        sh.sh_avail,
>        sh.slcolor,
>        sh.slminlen,
>        sh.slminlen * un.un_fact AS slminlen_cm,
>        sh.slmaxlen,
>        sh.slmaxlen * un.un_fact AS slmaxlen_cm,
>        sh.slunit
>       FROM shoe_data sh, unit un
>      WHERE sh.slunit = un.un_name;
>
>

I am not sure - this should by task for client application. But Pg
should have some pretty print function - it is easy implemented there.
Personally, I prefere Celko's notation, it is little bit more compact

SELECT sh.shoename, sh.sh_avail, sh.slcolor, sh.slminlen,
sh.slminlen * un.un_fact AS slminlen_cm, sh.slmaxlen,
sh.slmaxlen * un.un_fact AS slmaxlen_cm, sh.slunit
FROM shoe_data sh, unit un
WHERE sh.slunit = un.un_name;

but, sure - this is my personal preference.

> Is there any objection?

I thing so default should be unformated with some pretty printing support.

regards
Pavel Stehule

>
> cheers
>
> andrew
>
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 15:06:58
Message-ID: 4A954F92.5010405@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Pavel Stehule wrote:
> I am not sure - this should by task for client application.

pg_get_viewdef() already has a pretty print mode, and this change would
only affect output from that mode. Non-pretty printed output would be
unchanged.

My argument is that the pretty print mode for target lists is not at all
pretty.

I don't see why this has the be invented in every client. Then we'd have
to do it in psql, pg_dump and so on. If any client doesn't like our
pretty print output it can get the raw viewdef and do its own formatting.

> But Pg
> should have some pretty print function - it is easy implemented there.
> Personally, I prefere Celko's notation, it is little bit more compact
>
> SELECT sh.shoename, sh.sh_avail, sh.slcolor, sh.slminlen,
> sh.slminlen * un.un_fact AS slminlen_cm, sh.slmaxlen,
> sh.slmaxlen * un.un_fact AS slmaxlen_cm, sh.slunit
> FROM shoe_data sh, unit un
> WHERE sh.slunit = un.un_name;
>
> but, sure - this is my personal preference.
>

To do that we would need to keep track of how much space was used on the
line and how much space what we were adding would use. It's doable, but
it's a lot more work.

>
>> Is there any objection?
>>
>
> I thing so default should be unformated with some pretty printing support.
>
>
>

Please look at the function definition. You already have the option of
formatted or unformatted output.

cheers

andrew


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 16:12:26
Message-ID: 20090826161226.GE5065@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan wrote:
>
> [originally sent from wrong account :-( ]

Andrew, you can login to the majordomo site and set your secondary
address as an alias of this one. This means it'll recognize the other
address and allow you to post from there without going through the
moderator queue. Of course, that address will not receive any mail from
majordomo.

Note that if you do this, it will work automatically for all lists, not
just -hackers, so it is a lot better than subscribing to each list and
setting it "nomail".

It only takes a minute (but you need your Majordomo list password, which
can be emailed to you if you don't have it).
http://www.postgresql.org/mailpref/pgsql-hackers

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 16:19:02
Message-ID: 4A956076.9000209@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Alvaro Herrera wrote:
> Andrew Dunstan wrote:
>
>> [originally sent from wrong account :-( ]
>>
>
> Andrew, you can login to the majordomo site and set your secondary
> address as an alias of this one. This means it'll recognize the other
> address and allow you to post from there without going through the
> moderator queue. Of course, that address will not receive any mail from
> majordomo.
>
>

Thanks, that's one MD feature I didn't know about or had forgotten. Nice.

cheers

andrew


From: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 16:31:50
Message-ID: 4A956376.4050106@pse-consulting.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan wrote:
>
>
>> But Pg
>> should have some pretty print function - it is easy implemented there.
>> Personally, I prefere Celko's notation, it is little bit more compact
>>
>> SELECT sh.shoename, sh.sh_avail, sh.slcolor, sh.slminlen,
>> sh.slminlen * un.un_fact AS slminlen_cm, sh.slmaxlen,
>> sh.slmaxlen * un.un_fact AS slmaxlen_cm, sh.slunit
>> FROM shoe_data sh, unit un
>> WHERE sh.slunit = un.un_name;
>>
>> but, sure - this is my personal preference.
>>
>
>
> To do that we would need to keep track of how much space was used on
> the line and how much space what we were adding would use. It's
> doable, but it's a lot more work.

When initially implementing the pretty option, I ran into the same
consideration. Back then, I decided not to try any line breaking on the
column list. Instead, I treated the columns as "just a bunch of
columns", laying the emphasis on the from-clause (with potentially many
joined tables).
So a pretty column formatting should still be white-space saving.

Regards,
Andreas


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Andreas Pflug <pgadmin(at)pse-consulting(dot)de>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 17:40:35
Message-ID: 20090826174035.GG5065@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andreas Pflug escribió:

> When initially implementing the pretty option, I ran into the same
> consideration. Back then, I decided not to try any line breaking on the
> column list. Instead, I treated the columns as "just a bunch of
> columns", laying the emphasis on the from-clause (with potentially many
> joined tables).
> So a pretty column formatting should still be white-space saving.

It would be neat to have a way of detecting the client terminal's width
(psql knows it; it'd have to pass it as an additional parameter) and
output as many columns as fit on each line. This is a much more
invasive change though.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


From: decibel <decibel(at)decibel(dot)org>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 18:11:50
Message-ID: 967ECBA1-1D3E-47C5-8A1D-6ECF9B8EB9D8@decibel.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Aug 26, 2009, at 9:02 AM, Andrew Dunstan wrote:
> The tiny patch attached fixes a long-standing peeve of mine (and at
> least one of my clients'), namely that the target list printed in
> viewdefs are unreadable.
>
> example output now looks like this:
>
> regression=# select pg_get_viewdef('shoe',true);
> pg_get_viewdef
> -----------------------------------------------
> SELECT
> sh.shoename,
> sh.sh_avail,

Did we kill the idea of trying to retain the original view
definition? Granted, that doesn't really help for SELECT *...
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: decibel <decibel(at)decibel(dot)org>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 18:47:32
Message-ID: 4A958344.4060108@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

decibel wrote:
> On Aug 26, 2009, at 9:02 AM, Andrew Dunstan wrote:
>> The tiny patch attached fixes a long-standing peeve of mine (and at
>> least one of my clients'), namely that the target list printed in
>> viewdefs are unreadable.
>>
>> example output now looks like this:
>>
>> regression=# select pg_get_viewdef('shoe',true);
>> pg_get_viewdef
>> -----------------------------------------------
>> SELECT
>> sh.shoename,
>> sh.sh_avail,
>
>
> Did we kill the idea of trying to retain the original view definition?
> Granted, that doesn't really help for SELECT *...
>

That has nothing at all to do with the issue. The question is not about
whether to keep the original, it's about how to format the reconstructed
query.

cheers

andrew


From: Greg Stark <gsstark(at)mit(dot)edu>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 20:35:38
Message-ID: 407d949e0908261335w2db506d4k150b12f7328189a3@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Aug 26, 2009 at 7:47 PM, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
>> Did we kill the idea of trying to retain the original view definition?
>> Granted, that doesn't really help for SELECT *...
>
> That has nothing at all to do with the issue. The question is not about
> whether to keep the original, it's about how to format the reconstructed
> query.

I suspect Jim's thinking that if we keep the original we don't have to
reconstruct the query ever. Unfortunately cases like "select *" -- and
that's not the only case, think of columns that have been renamed --
throw a wrench in the works for that.

I agree with Tom's concerns -- think of that guy who was bumping up
against the 1600 column limit. At least if they're on one line you can
still see the structure of the query albeit with a very very wide
scrollbar...

But for typical queries I do agree one per line is better. That is
actually how I format my queries when they have complex expressions in
the target list. Perhaps formatting one per line whenever there's an
alias or the value is a complex expression but putting any unaliased
columns (such as produced by select *) in a single line would be a
good compromise?

Incidentally, how does your patch format a complex subquery in the target list?

but I think on balance this is probably better. In the extreme think
of that guy a few days ago who was bumping up against the 1600 column
limit. Assuming he had a few layers of nested subqueries his

--
greg
http://mit.edu/~gsstark/resume.pdf


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Greg Stark <gsstark(at)mit(dot)edu>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 22:07:18
Message-ID: 20355.1251324438@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Greg Stark <gsstark(at)mit(dot)edu> writes:
> I agree with Tom's concerns -- think of that guy who was bumping up
> against the 1600 column limit. At least if they're on one line you can
> still see the structure of the query albeit with a very very wide
> scrollbar...

> But for typical queries I do agree one per line is better. That is
> actually how I format my queries when they have complex expressions in
> the target list. Perhaps formatting one per line whenever there's an
> alias or the value is a complex expression but putting any unaliased
> columns (such as produced by select *) in a single line would be a
> good compromise?

Yeah, I was wondering about adopting some rule like that too.

It would be pretty easy to adjust that loop so that columns that aren't
simple Vars are put on their own line, while Vars are allowed to share
a line. I dunno whether users would see that as inconsistent, though.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 22:49:27
Message-ID: 4A95BBF7.7020301@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Greg Stark <gsstark(at)mit(dot)edu> writes:
>
>> I agree with Tom's concerns -- think of that guy who was bumping up
>> against the 1600 column limit. At least if they're on one line you can
>> still see the structure of the query albeit with a very very wide
>> scrollbar...
>>
>
>
>> But for typical queries I do agree one per line is better. That is
>> actually how I format my queries when they have complex expressions in
>> the target list. Perhaps formatting one per line whenever there's an
>> alias or the value is a complex expression but putting any unaliased
>> columns (such as produced by select *) in a single line would be a
>> good compromise?
>>
>
> Yeah, I was wondering about adopting some rule like that too.
>
> It would be pretty easy to adjust that loop so that columns that aren't
> simple Vars are put on their own line, while Vars are allowed to share
> a line. I dunno whether users would see that as inconsistent, though.
>

Yeah, probably, I don't like it much.

I do have a solution that wraps when running line length over 80 instead
of on every col:

SELECT sh.shoename, sh.sh_avail, sh.slcolor, sh.slminlen,
sh.slminlen * un.un_fact AS slminlen_cm, sh.slmaxlen,
sh.slmaxlen * un.un_fact AS slmaxlen_cm, sh.slunit
FROM shoe_data sh, unit un
WHERE sh.slunit = un.un_name;

It's not a huge amount of code.

Maybe we need a couple of extra pg_get_viewdef() variants. One to wrap
on some provided line length, one to wrap on every column. psql could
use the first, pg_dump could use the second.

I really can't believe anyone wants a single line with 1600 column specs ...

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 22:55:21
Message-ID: 23972.1251327321@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I do have a solution that wraps when running line length over 80 instead
> of on every col:

> SELECT sh.shoename, sh.sh_avail, sh.slcolor, sh.slminlen,
> sh.slminlen * un.un_fact AS slminlen_cm, sh.slmaxlen,
> sh.slmaxlen * un.un_fact AS slmaxlen_cm, sh.slunit
> FROM shoe_data sh, unit un
> WHERE sh.slunit = un.un_name;

> It's not a huge amount of code.

Well, let's see it? What do you do with expressions that don't fit?

> Maybe we need a couple of extra pg_get_viewdef() variants. One to wrap
> on some provided line length, one to wrap on every column. psql could
> use the first, pg_dump could use the second.

pg_dump doesn't use prettyprinting at all, and won't if I have anything
to say about it. But I could see teaching the psql \d views to pass
along whatever psql thinks the window width is.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 23:12:42
Message-ID: 4A95C16A.6090706@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> I do have a solution that wraps when running line length over 80 instead
>> of on every col:
>>
>
>
>> SELECT sh.shoename, sh.sh_avail, sh.slcolor, sh.slminlen,
>> sh.slminlen * un.un_fact AS slminlen_cm, sh.slmaxlen,
>> sh.slmaxlen * un.un_fact AS slmaxlen_cm, sh.slunit
>> FROM shoe_data sh, unit un
>> WHERE sh.slunit = un.un_name;
>>
>
>
>> It's not a huge amount of code.
>>
>
> Well, let's see it? What do you do with expressions that don't fit?
>

See attached.

We don't apply the wrapping unless there has been a column printed on
the line (except for the SELECT line).

So we can run over the limit on a line, but if we do there's only one
column spec. I think that's acceptable.

>
>> Maybe we need a couple of extra pg_get_viewdef() variants. One to wrap
>> on some provided line length, one to wrap on every column. psql could
>> use the first, pg_dump could use the second.
>>
>
> pg_dump doesn't use prettyprinting at all, and won't if I have anything
> to say about it. But I could see teaching the psql \d views to pass
> along whatever psql thinks the window width is.
>
>
>

OK, but I'd still like to have the only one col per line variant available.

cheers

andrew

Attachment Content-Type Size
prettyprint.patch text/x-patch 2.2 KB

From: Greg Stark <gsstark(at)mit(dot)edu>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-26 23:29:18
Message-ID: 407d949e0908261629y35020f8fi57c289db8a9b420a@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Aug 26, 2009 at 11:49 PM, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
> Maybe we need a couple of extra pg_get_viewdef() variants. One to wrap on
> some provided line length, one to wrap on every column. psql could use the
> first, pg_dump could use the second.
>
> I really can't believe anyone wants a single line with 1600 column specs ...

Uhm, why not? People generally don't care about the list of columns at
all if it's just generated by "select *". If they're reading it at all
it's to see the WHERE clauses and FROM clauses and so on.

I think wrapping it at 80 columns gets the worst of both worlds. Then
you have dozens or even hundreds of lines just listing columns making
it hard to see the rest of the query. At least if it's all on one line
you can just not scroll to the right and see the rest of the query on
your screen.

An alternative to my previous compromise which might be more consistent:

List the columns one per line if *any* of the columns has an alias or
is a complex expression. If they're all simple columns then put them
all one line.

At least that way you won't have any weird column lists that switch
back and forth between styles.

--
greg
http://mit.edu/~gsstark/resume.pdf


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Greg Stark <gsstark(at)mit(dot)edu>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 00:00:24
Message-ID: 4A95CC98.2070102@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Greg Stark wrote:
> At least if it's all on one line
> you can just not scroll to the right and see the rest of the query on
> your screen.
>
>
>

This is where the confusion arises.

This is not possible on any terminal program I use - they don't scroll
left and right, they wrap, and the result in this case is horrible.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 02:23:55
Message-ID: 26412.1251339835@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> Well, let's see it? What do you do with expressions that don't fit?

> See attached.

This isn't going to work as-is, because (a) buf->data can be moved
around by repalloc, and (b) you're not allowing for newlines being
introduced within the column expressions. You could probably fix it,
but given the lack of consensus for a line-length-based approach, I'm
not sure it's worth putting more effort into.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 02:30:15
Message-ID: 4A95EFB7.2050901@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Tom Lane wrote:
>>
>>> Well, let's see it? What do you do with expressions that don't fit?
>>>
>
>
>> See attached.
>>
>
> This isn't going to work as-is, because (a) buf->data can be moved
> around by repalloc, and (b) you're not allowing for newlines being
> introduced within the column expressions. You could probably fix it,
> but given the lack of consensus for a line-length-based approach, I'm
> not sure it's worth putting more effort into.
>

Yeah, it was just a prototype. I'll just provide for an pg_get_viewdef()
variant that adopts my original approach, which I don't think suffers
either of those problems. Surely that won't upset anyone, at least. It's
what I really wanted anyway.

cheers

andrew


From: Greg Stark <gsstark(at)mit(dot)edu>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 02:35:52
Message-ID: 407d949e0908261935s2ecbe22axc3478a937f200608@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Aug 27, 2009 at 1:00 AM, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
> Greg Stark wrote:
>>
>> At least if it's all on one line
>> you can just not scroll to the right and see the rest of the query on
>> your screen.
>
> This is where the confusion arises.
>
> This is not possible on any terminal program I use - they don't scroll left
> and right, they wrap, and the result in this case is horrible.

Well then you need a better terminal? Or you need to do your
programming in a text editor and not a terminal? Surely *any*
significant view definition will overflow on a terminal?

Incidentally I just tried
\d information_schema.views

and it *does* seem to put newlines after some of the target list
items. After each of the CASE expressions it puts a newline. So you
*already* get a mixture of some multiple items on a line and some
one-per-line.

So I think I'm back to my original suggestion, put any item with a
complex expression or an alias on a line by itself. Any plain column
names can be listed on a single line.

--
greg
http://mit.edu/~gsstark/resume.pdf


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Greg Stark <gsstark(at)mit(dot)edu>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 02:48:40
Message-ID: 26607.1251341320@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Greg Stark <gsstark(at)mit(dot)edu> writes:
> Incidentally I just tried
> \d information_schema.views

> and it *does* seem to put newlines after some of the target list
> items. After each of the CASE expressions it puts a newline. So you
> *already* get a mixture of some multiple items on a line and some
> one-per-line.

Yeah, sufficiently complex expressions (sub-selects, for an obvious
case) will get internal pretty-printing that might include newlines.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Greg Stark <gsstark(at)mit(dot)edu>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 03:14:54
Message-ID: 4A95FA2E.10606@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Greg Stark wrote:
> On Thu, Aug 27, 2009 at 1:00 AM, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
>
>> Greg Stark wrote:
>>
>>> At least if it's all on one line
>>> you can just not scroll to the right and see the rest of the query on
>>> your screen.
>>>
>> This is where the confusion arises.
>>
>> This is not possible on any terminal program I use - they don't scroll left
>> and right, they wrap, and the result in this case is horrible.
>>
>
> Well then you need a better terminal? Or you need to do your
> programming in a text editor and not a terminal?

I use emacs in its own window. And it too line wraps, although that can
be changed (I'm not a fan of line truncation mode, frankly). But I also
like to be able to read the viewdef, and I like to be able to cut and
paste it so it is sanely editable. And I'm not alone. If there are
embedded newlines in the column def that might be annoying, but what we
have now sucks majorly for anything but the most trivial of target lists.

cheers

andrew


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 14:26:07
Message-ID: 4A96977F.4010700@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Greg Stark <gsstark(at)mit(dot)edu> writes:
>
>> Incidentally I just tried
>> \d information_schema.views
>>
>
>
>> and it *does* seem to put newlines after some of the target list
>> items. After each of the CASE expressions it puts a newline. So you
>> *already* get a mixture of some multiple items on a line and some
>> one-per-line.
>>
>
> Yeah, sufficiently complex expressions (sub-selects, for an obvious
> case) will get internal pretty-printing that might include newlines.
>

OK, drawing this together, what I did was to go back closer to my
original idea, but put this in a separate function, so nobody would get
too upset ;-)

Here is what my function does, and also what the current "pretty
printing" does:

andrew=# select pg_get_viewdef_long('foo');
pg_get_viewdef_long
------------------------------
SELECT 'a'::text AS b,
( SELECT 1
FROM dual) AS x,
random() AS y,
CASE
WHEN true THEN 1
ELSE 0
END AS c,
1 AS d
FROM dual;
(1 row)

andrew=# select pg_get_viewdef('foo',true);
pg_get_viewdef
---------------------------------------------
SELECT 'a'::text AS b, ( SELECT 1
FROM dual) AS x, random() AS y,
CASE
WHEN true THEN 1
ELSE 0
END AS c, 1 AS d
FROM dual;
(1 row)

WIP Patch is attached. To complete it I would add a psql option to use
it, but maybe we should have a psql setting to enable it ... building
something extra into the \d* stuff looks a bit ugly, since we already
have a million options.

cheers

andrew

Attachment Content-Type Size
prettyprint.patch text/x-patch 5.7 KB

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 16:17:19
Message-ID: 5297.1251389839@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> OK, drawing this together, what I did was to go back closer to my
> original idea, but put this in a separate function, so nobody would get
> too upset ;-)

This seems seriously ugly. Why don't you have the flag just driving
your original two-line addition?

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 16:30:55
Message-ID: 4A96B4BF.90309@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> OK, drawing this together, what I did was to go back closer to my
>> original idea, but put this in a separate function, so nobody would get
>> too upset ;-)
>>
>
> This seems seriously ugly. Why don't you have the flag just driving
> your original two-line addition?
>
>
>

I am confused.

The original two line addition was already in effect driven by the
PRETTY_INDENT flag, because the appendContextKeyword call would be
effectively a no-op if that flag wasn't on. But apparently some people
don't want each column on a separate line, as I do, even when it's
pretty printed, so, since that's what I want, I provided for it in a
separate function, but I made the code take account of the cases you and
Greg mentioned, where it already begins a new line for the column def.

So, what exactly is ugly? My code? I can believe that. I have since made
it slightly simpler by using a pstrdup'ed string instead of an extra
StringInfo object. The output? That's a matter of taste, but I don't see
how it's less ugly than what's there now. The idea of a new function? I
don't see how to get what I want without it unless we're prepared to
upset some of the people who have objected to my proposal.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 17:08:55
Message-ID: 6055.1251392935@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I am confused.

> The original two line addition was already in effect driven by the
> PRETTY_INDENT flag, because the appendContextKeyword call would be
> effectively a no-op if that flag wasn't on. But apparently some people
> don't want each column on a separate line, as I do, even when it's
> pretty printed, so, since that's what I want, I provided for it in a
> separate function, but I made the code take account of the cases you and
> Greg mentioned, where it already begins a new line for the column def.

What I was imagining was simply providing an additional pretty-print
flag that gives the alternatives of the current behavior, or the patch
you originally proposed that adds newlines between targetlist items all
the time. I don't think that you should complicate the behavior any
more than that.

Personally I would prefer the original patch to this one.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 18:04:00
Message-ID: 4A96CA90.80002@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> I am confused.
>>
>
>
>> The original two line addition was already in effect driven by the
>> PRETTY_INDENT flag, because the appendContextKeyword call would be
>> effectively a no-op if that flag wasn't on. But apparently some people
>> don't want each column on a separate line, as I do, even when it's
>> pretty printed, so, since that's what I want, I provided for it in a
>> separate function, but I made the code take account of the cases you and
>> Greg mentioned, where it already begins a new line for the column def.
>>
>
> What I was imagining was simply providing an additional pretty-print
> flag that gives the alternatives of the current behavior, or the patch
> you originally proposed that adds newlines between targetlist items all
> the time. I don't think that you should complicate the behavior any
> more than that.
>
> Personally I would prefer the original patch to this one.
>
>
>

OK, and how are we going to set that flag? Like I did, with a separate
function?

I assume you are in effect saying you don't mind if there is an
occasional blank line in the output.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 18:17:22
Message-ID: 20874.1251397042@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> OK, and how are we going to set that flag? Like I did, with a separate
> function?

I would be inclined to invent a variant of pg_get_viewdef with an
additional parameter rather than choosing a new function name, but
otherwise yeah. Or we could decide this isn't worth all the
trouble and just go back to your original patch. By the time you
get done with all the documentation and client-side hacking that
would be required, this patch is going to be a lot larger than it
seems worth.

> I assume you are in effect saying you don't mind if there is an
> occasional blank line in the output.

What blank line? I would expect prettyprinting of expressions to
sometimes insert an embedded newline, but not one at the beginning
or end. Do you have a counterexample?

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2009-08-27 18:37:02
Message-ID: 4A96D24E.6040201@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
>> I assume you are in effect saying you don't mind if there is an
>> occasional blank line in the output.
>>
>
> What blank line? I would expect prettyprinting of expressions to
> sometimes insert an embedded newline, but not one at the beginning
> or end. Do you have a counterexample?
>
>
>

Yes, CASE expressions, as in my previously posted example:

SELECT 'a'::text AS b, ( SELECT 1
FROM dual) AS x, random() AS y,
CASE
WHEN true THEN 1
ELSE 0
END AS c, 1 AS d
FROM dual;

cheers

andrew


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2010-02-23 05:52:24
Message-ID: 201002230552.o1N5qOh09681@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


What happened to this? I didn't see it applied.

---------------------------------------------------------------------------

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> > OK, and how are we going to set that flag? Like I did, with a separate
> > function?
>
> I would be inclined to invent a variant of pg_get_viewdef with an
> additional parameter rather than choosing a new function name, but
> otherwise yeah. Or we could decide this isn't worth all the
> trouble and just go back to your original patch. By the time you
> get done with all the documentation and client-side hacking that
> would be required, this patch is going to be a lot larger than it
> seems worth.
>
> > I assume you are in effect saying you don't mind if there is an
> > occasional blank line in the output.
>
> What blank line? I would expect prettyprinting of expressions to
> sometimes insert an embedded newline, but not one at the beginning
> or end. Do you have a counterexample?
>
> regards, tom lane
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2010-02-23 15:59:10
Message-ID: 4B83FB4E.90600@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Bruce Momjian wrote:
> What happened to this? I didn't see it applied.
>
>

I got puzzled by some delphic comments, and then I got pulled into work
of a higher priority, so it slipped down my list.

Maybe we can pick it up again in 9.1.

cheers

andrew


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2010-02-23 16:18:10
Message-ID: 201002231618.o1NGIBQ24166@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan wrote:
>
>
>
> Bruce Momjian wrote:
> > What happened to this? I didn't see it applied.
> >
> >
>
> I got puzzled by some delphic comments, and then I got pulled into work
> of a higher priority, so it slipped down my list.
>
> Maybe we can pick it up again in 9.1.

OK, should it be added to the TODO list?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Greg Stark <gsstark(at)mit(dot)edu>, decibel <decibel(at)decibel(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pretty print viewdefs
Date: 2010-02-23 16:30:36
Message-ID: 4B8402AC.9020809@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Bruce Momjian wrote:
> Andrew Dunstan wrote:
>
>>
>> Bruce Momjian wrote:
>>
>>> What happened to this? I didn't see it applied.
>>>
>>>
>>>
>> I got puzzled by some delphic comments, and then I got pulled into work
>> of a higher priority, so it slipped down my list.
>>
>> Maybe we can pick it up again in 9.1.
>>
>
>
> OK, should it be added to the TODO list?
>
>

Sure.

cheers

andrew