pretty print viewdefs

Andrew Dunstan <andrew@dunslane.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

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.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

Andrew Dunstan <andrew@dunslane.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

2009/8/26 Andrew Dunstan <andrew@dunslane.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

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

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.

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

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

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.

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@decibel.org
Give your computer some brain candy! www.distributed.net Team #1828

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

On Wed, Aug 26, 2009 at 7:47 PM, Andrew Dunstan<andrew@dunslane.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

Greg Stark <gsstark@mit.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

Tom Lane wrote:

Greg Stark <gsstark@mit.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

Andrew Dunstan <andrew@dunslane.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

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.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

On Wed, Aug 26, 2009 at 11:49 PM, Andrew Dunstan<andrew@dunslane.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

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

Andrew Dunstan <andrew@dunslane.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