Thread

psql describe.c cleanup

Josh Kupershmidt <schmiddy@gmail.com> — 2011-05-07T19:40:35Z

Hi all,

I use psql's -E mode every now and then, copy-and-pasting and further
tweaking the SQL displayed. Most queries are displayed terminated by a
semicolon, but quite a few aren't, making copy-and-paste just a bit
more tedious.

Attached is a patch to fix every SQL query I saw in describe.c. There
were also a few queries with trailing newlines, and I fixed those too.

And while I'm griping about describe.c, is it just me or is the source
code indentation in that file totally screwy? I'm using emacs and I've
loaded the snippet for pgsql-c-mode from
./src/tools/editors/emacs.samples into my ~/.emacs, but the
indentation of multiline strings still looks inconsistent. See
attached screenshot, e.g. the start of line 80 has four tabs and one
space, while line 79 has six tabs and two spaces?

Josh

Re: psql describe.c cleanup

Peter Eisentraut <peter_e@gmx.net> — 2011-05-07T20:50:46Z

On lör, 2011-05-07 at 15:40 -0400, Josh Kupershmidt wrote:
> And while I'm griping about describe.c, is it just me or is the source
> code indentation in that file totally screwy? I'm using emacs and I've
> loaded the snippet for pgsql-c-mode from
> ./src/tools/editors/emacs.samples into my ~/.emacs, but the
> indentation of multiline strings still looks inconsistent. See
> attached screenshot, e.g. the start of line 80 has four tabs and one
> space, while line 79 has six tabs and two spaces?

Yes, it's screwy, but not only there.  See

http://archives.postgresql.org/message-id/1295913661.13617.5.camel@vanquo.pezone.net

for the reason.

Re: psql describe.c cleanup

Merlin Moncure <mmoncure@gmail.com> — 2011-06-14T16:15:54Z

On Sat, May 7, 2011 at 2:40 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
> Hi all,
>
> I use psql's -E mode every now and then, copy-and-pasting and further
> tweaking the SQL displayed. Most queries are displayed terminated by a
> semicolon, but quite a few aren't, making copy-and-paste just a bit
> more tedious.
>
> Attached is a patch to fix every SQL query I saw in describe.c. There
> were also a few queries with trailing newlines, and I fixed those too.

I did a quick review and test of your patch.  It didn't quite apply
cleanly due to recent non-related describe.c changes -- updated patch
attached.

First, I'll give you a thumbs up on the original inspiration for the
patch.  The output should be standardized, and I see no reason not to
append a semicolon on usability basis.  Beyond that, the changes are
mostly cosmetic and I can't see how it will break things outside of
terminating a query early by accident (I didn't see any).

What I do wonder though is if the ; appending should really be
happening in printQuery() instead of in each query -- the idea being
that formatting for external consumption should be happening in one
place.  Maybe that's over-thinking it though.

merlin

Re: psql describe.c cleanup

Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-14T19:25:14Z

Excerpts from Josh Kupershmidt's message of sáb may 07 16:40:35 -0300 2011:

> And while I'm griping about describe.c, is it just me or is the source
> code indentation in that file totally screwy? I'm using emacs and I've
> loaded the snippet for pgsql-c-mode from
> ./src/tools/editors/emacs.samples into my ~/.emacs, but the
> indentation of multiline strings still looks inconsistent. See
> attached screenshot, e.g. the start of line 80 has four tabs and one
> space, while line 79 has six tabs and two spaces?

pgindent moves strings back to the left when it thinks they fit within
80 columns.  Yes, that seems pretty screwy.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: psql describe.c cleanup

Josh Kupershmidt <schmiddy@gmail.com> — 2011-06-15T02:08:40Z

On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure <mmoncure@gmail.com> wrote:
> I did a quick review and test of your patch.  It didn't quite apply
> cleanly due to recent non-related describe.c changes -- updated patch
> attached.

Thanks for looking at this. Your updated patch looks good to me.

> First, I'll give you a thumbs up on the original inspiration for the
> patch.  The output should be standardized, and I see no reason not to
> append a semicolon on usability basis.  Beyond that, the changes are
> mostly cosmetic and I can't see how it will break things outside of
> terminating a query early by accident (I didn't see any).

Yeah, I really didn't want to break any queries, so I did my best to
test every query I changed.

> What I do wonder though is if the ; appending should really be
> happening in printQuery() instead of in each query -- the idea being
> that formatting for external consumption should be happening in one
> place.  Maybe that's over-thinking it though.

That's a fair point, and hacking up printQuery() would indeed solve my
original gripe about copy-pasting queries out of psql -E. But more
generally, I think that standardizing the style of the code is a Good
Thing, particularly where style issues impact usability (like here). I
would actually like to see a movement towards having all these queries
use whitespace/formatting consistently. For instance, when I do a

  \d sometable

I see some queries with lines bleeding out maybe 200 columns, some
wrapped at 80 columns, etc. This sort of style issue is not something
that a simple kludge in printQuery() could solve (and even if we put
in a sophisticated formatting tool inside printQuery(), we'd still be
left with an ugly describe.c). For the record, I find that having SQL
queries consistently formatted makes them much more readable, allowing
a human to roughly "parse" a query on sight once you're used to the
formatting.

So I wouldn't be opposed to putting the kludge in printQuery(), but I
would like to see us standardize the queries regardless. (And in that
case, I wouldn't mind if we dropped all the semicolons in describe.c,
just that we're consistent.)

Josh

Re: psql describe.c cleanup

Josh Kupershmidt <schmiddy@gmail.com> — 2011-06-15T02:11:43Z

On Tue, Jun 14, 2011 at 3:25 PM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
> pgindent moves strings back to the left when it thinks they fit within
> 80 columns.  Yes, that seems pretty screwy.

I am losing track of the ways in which pgindent has managed to mangle
our source code :-/

Josh

Re: psql describe.c cleanup

Merlin Moncure <mmoncure@gmail.com> — 2011-06-15T14:01:50Z

On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
> On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure <mmoncure@gmail.com> wrote:
>> What I do wonder though is if the ; appending should really be
>> happening in printQuery() instead of in each query -- the idea being
>> that formatting for external consumption should be happening in one
>> place.  Maybe that's over-thinking it though.
>
> That's a fair point, and hacking up printQuery() would indeed solve my
> original gripe about copy-pasting queries out of psql -E. But more
> generally, I think that standardizing the style of the code is a Good
> Thing, particularly where style issues impact usability (like here).

sure -- if anyone would like to comment on this one way or the other
feel free -- otherwise I'll pass the patch up the chain as-is...it's
not exactly the 'debate of the century' :-).

merlin

Re: psql describe.c cleanup

Merlin Moncure <mmoncure@gmail.com> — 2011-06-16T14:05:30Z

On Wed, Jun 15, 2011 at 9:01 AM, Merlin Moncure <mmoncure@gmail.com> wrote:
> On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
>> On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure <mmoncure@gmail.com> wrote:
>>> What I do wonder though is if the ; appending should really be
>>> happening in printQuery() instead of in each query -- the idea being
>>> that formatting for external consumption should be happening in one
>>> place.  Maybe that's over-thinking it though.
>>
>> That's a fair point, and hacking up printQuery() would indeed solve my
>> original gripe about copy-pasting queries out of psql -E. But more
>> generally, I think that standardizing the style of the code is a Good
>> Thing, particularly where style issues impact usability (like here).
>
> sure -- if anyone would like to comment on this one way or the other
> feel free -- otherwise I'll pass the patch up the chain as-is...it's
> not exactly the 'debate of the century' :-).

done.  marked ready for committer.

merlin

Re: psql describe.c cleanup

Robert Haas <robertmhaas@gmail.com> — 2011-07-06T14:14:04Z

On Thu, Jun 16, 2011 at 10:05 AM, Merlin Moncure <mmoncure@gmail.com> wrote:
> On Wed, Jun 15, 2011 at 9:01 AM, Merlin Moncure <mmoncure@gmail.com> wrote:
>> On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
>>> On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure <mmoncure@gmail.com> wrote:
>>>> What I do wonder though is if the ; appending should really be
>>>> happening in printQuery() instead of in each query -- the idea being
>>>> that formatting for external consumption should be happening in one
>>>> place.  Maybe that's over-thinking it though.
>>>
>>> That's a fair point, and hacking up printQuery() would indeed solve my
>>> original gripe about copy-pasting queries out of psql -E. But more
>>> generally, I think that standardizing the style of the code is a Good
>>> Thing, particularly where style issues impact usability (like here).
>>
>> sure -- if anyone would like to comment on this one way or the other
>> feel free -- otherwise I'll pass the patch up the chain as-is...it's
>> not exactly the 'debate of the century' :-).
>
> done.  marked ready for committer.

Committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company