Thread: Patch to change psql default banner

Patch to change psql default banner

From
"Joshua D. Drake"
Date:
Hello,

As discussed:

http://archives.postgresql.org/pgsql-hackers/2008-04/msg01476.php

The patch does the following:

Adds an Execution line to the \? output.
Changes the help output in mainloop.c to be more useful.
Greatly reduces overall default banner output:
 * shows client version and type help for help only
   * if server doesn't match shows server version too
 * if there is a major version mismatch it throws a warning

Sincerely,

Joshua D. Drake


--
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



Attachment

Re: Patch to change psql default banner v6

From
"Joshua D. Drake"
Date:
On Wed, 23 Apr 2008 14:41:20 -0700
"Joshua D. Drake" <jd@commandprompt.com> wrote:

Hello,

Per final discussion here:

http://archives.postgresql.org/pgsql-hackers/2008-04/msg01607.php

Joshua D. Drake


--
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



Attachment

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Joshua D. Drake wrote:
> On Wed, 23 Apr 2008 14:41:20 -0700
> "Joshua D. Drake" <jd@commandprompt.com> wrote:
>
> Hello,
>
> Per final discussion here:
>
> http://archives.postgresql.org/pgsql-hackers/2008-04/msg01607.php

Isn't this going to mean \g is listed twice?

    +   fprintf(output, _("Execution\n"));
    +   fprintf(output, _("  \\g or ;    execute query\n\n"));

If you want I can look at reorganizing the \? help.  I have a larger
reorganization mind.

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

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

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Joshua D. Drake wrote:
> On Wed, 23 Apr 2008 14:41:20 -0700
> "Joshua D. Drake" <jd@commandprompt.com> wrote:
>
> Hello,
>
> Per final discussion here:
>
> http://archives.postgresql.org/pgsql-hackers/2008-04/msg01607.php

I have looked over this patch and made a few adjustments.

You used for a help startup banner:

    Type: help for help.

I feel this has too much indirection because 'help' then produced:

    You are using psql, the command-line interface to PostgreSQL.

        \h or \\help for SQL help.
        \? for psql help.
        \q to quit psql.

        \copyright to view the copyright.

Because \? now has \h, \q, and \copyright alone at the top I think we
should just use:

    $ psql test
    psql (8.4devel)   Type \? for help.

    test=>

If you type 'help' it just repeats the startup banner suggestion:

    test=> help

    You are using psql, the command-line interface to PostgreSQL.
    Type \? for help.

    test=>

I think that consistency will be clearer.  In the past we were trying to
avoid \?, but I think now it is clean enough to be used by new people
without confusion.

I also put the version number in parentheses so it wouldn't be as
prominent.

Updated patch attached.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/help.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.127
diff -c -c -r1.127 help.c
*** src/bin/psql/help.c    14 May 2008 15:30:22 -0000    1.127
--- src/bin/psql/help.c    14 May 2008 23:09:06 -0000
***************
*** 176,182 ****

      fprintf(output, _("Query Buffer\n"));
      fprintf(output, _("  \\e [FILE]      edit the query buffer (or file) with external editor\n"));
!     fprintf(output, _("  \\g [FILE]      send query buffer to server (and results to file or |pipe)\n"));
      fprintf(output, _("  \\p             show the contents of the query buffer\n"));
      fprintf(output, _("  \\r             reset (clear) the query buffer\n"));
  #ifdef USE_READLINE
--- 176,182 ----

      fprintf(output, _("Query Buffer\n"));
      fprintf(output, _("  \\e [FILE]      edit the query buffer (or file) with external editor\n"));
!     fprintf(output, _("  \\g [FILE] or ; send query buffer to server (and results to file or |pipe)\n"));
      fprintf(output, _("  \\p             show the contents of the query buffer\n"));
      fprintf(output, _("  \\r             reset (clear) the query buffer\n"));
  #ifdef USE_READLINE
Index: src/bin/psql/mainloop.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v
retrieving revision 1.90
diff -c -c -r1.90 mainloop.c
*** src/bin/psql/mainloop.c    5 Apr 2008 03:40:15 -0000    1.90
--- src/bin/psql/mainloop.c    14 May 2008 23:09:06 -0000
***************
*** 177,186 ****
              (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
          {
              free(line);
!             puts(_("You are using psql, the command-line interface to PostgreSQL."));
!             puts(_("Enter SQL commands, or type \\? for a list of backslash options."));
!             puts(_("Use \\h for SQL command help."));
!             puts(_("Use \\q to quit."));
              fflush(stdout);
              continue;
          }
--- 177,185 ----
              (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
          {
              free(line);
!             puts(_("\nYou are using psql, the command-line interface to PostgreSQL."));
!             puts(_("Type \\? for help.\n"));
!
              fflush(stdout);
              continue;
          }
Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.147
diff -c -c -r1.147 startup.c
*** src/bin/psql/startup.c    8 May 2008 17:04:26 -0000    1.147
--- src/bin/psql/startup.c    14 May 2008 23:09:06 -0000
***************
*** 317,342 ****
                      server_version = server_ver_str;
                  }

!                 printf(_("Welcome to %s %s (server %s), the PostgreSQL interactive terminal.\n\n"),
!                        pset.progname, PG_VERSION, server_version);
              }
              else
!                 printf(_("Welcome to %s %s, the PostgreSQL interactive terminal.\n\n"),
!                        pset.progname, PG_VERSION);
!
!             printf(_("Type:  \\copyright for distribution terms\n"
!                      "       \\h for help with SQL commands\n"
!                      "       \\? for help with psql commands\n"
!                   "       \\g or terminate with semicolon to execute query\n"
!                      "       \\q to quit\n\n"));

              if (pset.sversion / 100 != client_ver / 100)
!                 printf(_("WARNING:  You are connected to a server with major version %d.%d,\n"
!                          "but your %s client is major version %d.%d.  Some backslash commands,\n"
!                          "such as \\d, might not work properly.\n\n"),
!                        pset.sversion / 10000, (pset.sversion / 100) % 100,
!                        pset.progname,
!                        client_ver / 10000, (client_ver / 100) % 100);

  #ifdef USE_SSL
              printSSLInfo();
--- 317,335 ----
                      server_version = server_ver_str;
                  }

!                 printf(_("\n\t%s (%s, server %s)"),
!                 pset.progname, PG_VERSION, server_version);
              }
              else
!                 printf("%s (%s)", pset.progname, PG_VERSION);

              if (pset.sversion / 100 != client_ver / 100)
!                 printf(_("\tWARNING: Server version %d.%d, %s version %d.%d.\n"
!                      "\tSome psql features may not work.\n\n"),
!                     pset.sversion / 10000, (pset.sversion / 100) % 100,
!                     pset.progname, client_ver / 10000, (client_ver / 100) % 100);
!
!             printf(_("   Type \\? for help.\n\n"));

  #ifdef USE_SSL
              printSSLInfo();

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
> I think that consistency will be clearer.  In the past we were trying to
> avoid \?, but I think now it is clean enough to be used by new people
> without confusion.
>
> I also put the version number in parentheses so it wouldn't be as
> prominent.
>
> Updated patch attached.

FYI, after the patch is applied I will update psql banner examples in
our documentation.

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

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

Re: Patch to change psql default banner v6

From
Alvaro Herrera
Date:
Bruce Momjian wrote:

> If you type 'help' it just repeats the startup banner suggestion:
>
>     test=> help
>
>     You are using psql, the command-line interface to PostgreSQL.
>     Type \? for help.

I think we wanted to have more information in 'help', not less.  Making
it just repeat the startup info is not very helpful.

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

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> Bruce Momjian wrote:
>
> > If you type 'help' it just repeats the startup banner suggestion:
> >
> >     test=> help
> >
> >     You are using psql, the command-line interface to PostgreSQL.
> >     Type \? for help.
>
> I think we wanted to have more information in 'help', not less.  Making
> it just repeat the startup info is not very helpful.

I thought about that, but aren't we just repeating the top of \?.  Is
that helpful?  Should we just display \?.

I know we decided not to do that, but I am trying to figure out what the
goal if 'help' is?  To display the most frequently-used help commands?
Aren't they at the top of \?.

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

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

Re: Patch to change psql default banner v6

From
"Joshua D. Drake"
Date:
Bruce Momjian wrote:
> Alvaro Herrera wrote:
>> Bruce Momjian wrote:
>>
>>> If you type 'help' it just repeats the startup banner suggestion:
>>>
>>>     test=> help
>>>
>>>     You are using psql, the command-line interface to PostgreSQL.
>>>     Type \? for help.
>> I think we wanted to have more information in 'help', not less.  Making
>> it just repeat the startup info is not very helpful.
>
> I thought about that, but aren't we just repeating the top of \?.  Is
> that helpful?  Should we just display \?.

I am a little confused here. The whole point of this patch is to remove
all extraneous information from the startup banner and push it to a
"help" screen. Thus type help for help.

If you type help you get your help options.

>
> I know we decided not to do that, but I am trying to figure out what the
> goal if 'help' is?  To display the most frequently-used help commands?
> Aren't they at the top of \?.
>

The purpose of help is to provide what help options there are available
to those who need them.

Joshua D. Drake


Re: Patch to change psql default banner v6

From
Alvaro Herrera
Date:
Bruce Momjian wrote:

> I know we decided not to do that, but I am trying to figure out what the
> goal if 'help' is?  To display the most frequently-used help commands?
> Aren't they at the top of \?.

The purpose of 'help' is to provide useful help.  If it only says "see \?"
then it's just redirecting you somewhere else, which isn't useful.

I don't think the various help commands need to be completely
orthogonal.  If you agree with that, then making 'help' repeat part of
what \? says is acceptable.  (Of course, the idea is not just to repeat,
but also to provide useful advice to the unwary.)

Remember, the people who is going to type 'help' is not the 10-year-Pg-
experience types.  It's the newbies.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> Bruce Momjian wrote:
>
> > I know we decided not to do that, but I am trying to figure out what the
> > goal if 'help' is?  To display the most frequently-used help commands?
> > Aren't they at the top of \?.
>
> The purpose of 'help' is to provide useful help.  If it only says "see \?"
> then it's just redirecting you somewhere else, which isn't useful.
>
> I don't think the various help commands need to be completely
> orthogonal.  If you agree with that, then making 'help' repeat part of
> what \? says is acceptable.  (Of course, the idea is not just to repeat,
> but also to provide useful advice to the unwary.)

OK.

> Remember, the people who is going to type 'help' is not the 10-year-Pg-
> experience types.  It's the newbies.

The larger issue is whether we want to advertise only "help" in the
startup banner.  The patch has just:

    Type: help for help.

Now, aside from being confusing (we need quotes around "help"), I
thought we should only mention \?.

My question is whether we agreed that suggesting "help" as the best way
to get help was what we agreed upon?  If we did, I forgot.  I thought
the 'help' ideas was just for people who forgot the help commands.

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

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

Re: Patch to change psql default banner v6

From
Alvaro Herrera
Date:
Bruce Momjian wrote:

> My question is whether we agreed that suggesting "help" as the best way
> to get help was what we agreed upon?  If we did, I forgot.  I thought
> the 'help' ideas was just for people who forgot the help commands.

Please review the previous discussion:

http://archives.postgresql.org/message-id/1200851790.19135.68.camel%40greg-laptop

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> Bruce Momjian wrote:
>
> > My question is whether we agreed that suggesting "help" as the best way
> > to get help was what we agreed upon?  If we did, I forgot.  I thought
> > the 'help' ideas was just for people who forgot the help commands.
>
> Please review the previous discussion:
>
> http://archives.postgresql.org/message-id/1200851790.19135.68.camel%40greg-laptop

OK, I just read the thread and saw no one say we should be promoting
_only_ 'help' in the startup banner.  Where is that email discussion?

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

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

Re: Patch to change psql default banner v6

From
"Joshua D. Drake"
Date:
Bruce Momjian wrote:
> Alvaro Herrera wrote:
>> Bruce Momjian wrote:
>>
>>> My question is whether we agreed that suggesting "help" as the best way
>>> to get help was what we agreed upon?  If we did, I forgot.  I thought
>>> the 'help' ideas was just for people who forgot the help commands.
>> Please review the previous discussion:
>>
>> http://archives.postgresql.org/message-id/1200851790.19135.68.camel%40greg-laptop
>
> OK, I just read the thread and saw no one say we should be promoting
> _only_ 'help' in the startup banner.  Where is that email discussion?
>

http://archives.postgresql.org/pgsql-hackers/2008-04/msg01476.php

And most specifically:

http://archives.postgresql.org/pgsql-hackers/2008-04/msg01376.php

Joshua D. Drake

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Joshua D. Drake wrote:
> Bruce Momjian wrote:
> > Alvaro Herrera wrote:
> >> Bruce Momjian wrote:
> >>
> >>> My question is whether we agreed that suggesting "help" as the best way
> >>> to get help was what we agreed upon?  If we did, I forgot.  I thought
> >>> the 'help' ideas was just for people who forgot the help commands.
> >> Please review the previous discussion:
> >>
> >> http://archives.postgresql.org/message-id/1200851790.19135.68.camel%40greg-laptop
> >
> > OK, I just read the thread and saw no one say we should be promoting
> > _only_ 'help' in the startup banner.  Where is that email discussion?
> >
>
> http://archives.postgresql.org/pgsql-hackers/2008-04/msg01476.php
>
> And most specifically:
>
> http://archives.postgresql.org/pgsql-hackers/2008-04/msg01376.php

Ah, OK.  I had forgotten.  Here is the new output:

    $ sql test
    psql (8.4devel)   Type "help" for help.

    test=> help

    You are using psql, the command-line interface to PostgreSQL.
            \h or \help for SQL help
            \? for psql help
            \q to quit psql

            \copyright to view the copyright

    test=> \?
    General
      \copyright     show PostgreSQL usage and distribution terms
      \g [FILE] or ; send query buffer to server (and results to file or |pipe)
      \h [NAME]      help on syntax of SQL commands, * for all commands
      \q             quit psql

    Query Buffer
      \e [FILE]      edit the query buffer (or file) with external editor
    ...

I moved '\g' up into the "General" section rather than make it a
single-entry section.

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

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

Re: Patch to change psql default banner v6

From
"Joshua D. Drake"
Date:
Bruce Momjian wrote:

>
>     test=> \?
>     General
>       \copyright     show PostgreSQL usage and distribution terms
>       \g [FILE] or ; send query buffer to server (and results to file or |pipe)
>       \h [NAME]      help on syntax of SQL commands, * for all commands
>       \q             quit psql
>
>     Query Buffer
>       \e [FILE]      edit the query buffer (or file) with external editor
>     ...
>
> I moved '\g' up into the "General" section rather than make it a
> single-entry section.

"send query buffer to server means nothing to a newbie". You execute
queries, you don't send buffers (from a user perspective).

Sincerely,

Joshua D. Drake


Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Joshua D. Drake wrote:
> Bruce Momjian wrote:
>
> >
> >     test=> \?
> >     General
> >       \copyright     show PostgreSQL usage and distribution terms
> >       \g [FILE] or ; send query buffer to server (and results to file or |pipe)
> >       \h [NAME]      help on syntax of SQL commands, * for all commands
> >       \q             quit psql
> >
> >     Query Buffer
> >       \e [FILE]      edit the query buffer (or file) with external editor
> >     ...
> >
> > I moved '\g' up into the "General" section rather than make it a
> > single-entry section.
>
> "send query buffer to server means nothing to a newbie". You execute
> queries, you don't send buffers (from a user perspective).

Yep, good, updated:

    General
      \copyright     show PostgreSQL usage and distribution terms
      \g [FILE] or ; execute query (and send results to file or |pipe)
      \h [NAME]      help on syntax of SQL commands, * for all commands
      \q             quit psql


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

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

Re: Patch to change psql default banner v6

From
"Joshua D. Drake"
Date:
Bruce Momjian wrote:
> Joshua D. Drake wrote:
>> Bruce Momjian wrote:

>>> I moved '\g' up into the "General" section rather than make it a
>>> single-entry section.
>> "send query buffer to server means nothing to a newbie". You execute
>> queries, you don't send buffers (from a user perspective).
>
> Yep, good, updated:
>
>     General
>       \copyright     show PostgreSQL usage and distribution terms
>       \g [FILE] or ; execute query (and send results to file or |pipe)
>       \h [NAME]      help on syntax of SQL commands, * for all commands
>       \q             quit psql
>
>

Cool.

Joshua D. Drake

Re: Patch to change psql default banner v6

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Ah, OK.  I had forgotten.  Here is the new output:

>     $ sql test
>     psql (8.4devel)   Type "help" for help.

>     test=> help

You are being unreasonably cryptic here.  What happens when there
is optional output --- ie, version mismatch warning and/or SSL info?

            regards, tom lane

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Ah, OK.  I had forgotten.  Here is the new output:
>
> >     $ sql test
> >     psql (8.4devel)   Type "help" for help.
>
> >     test=> help
>
> You are being unreasonably cryptic here.  What happens when there
> is optional output --- ie, version mismatch warning and/or SSL info?

Oh, good point.  Let me look at that.  Thanks.  You prefer:

    $ sql test
    psql (8.4devel)
    Type "help" for help.

    test=> help

That looked so sparse to me.

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

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

Re: Patch to change psql default banner v6

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Oh, good point.  Let me look at that.  Thanks.  You prefer:

>     $ sql test
>     psql (8.4devel)
>     Type "help" for help.

>     test=> help

Well, the question is still "where is the optional info going to go?"

I think what I'd find nice looking is

    $ psql test
    psql 8.4devel  [ server version warning here, if needed ]
    [ line with SSL info here, if needed ]
    Type "help" for help.

    test=>

I do feel that the help statement ought to be on its own line;
the other way is going to look cluttered, particularly as soon
as there's a version warning in there.

            regards, tom lane

Re: Patch to change psql default banner v6

From
"Joshua D. Drake"
Date:
Tom Lane wrote:

> Well, the question is still "where is the optional info going to go?"
>
> I think what I'd find nice looking is
>
>     $ psql test
>     psql 8.4devel  [ server version warning here, if needed ]
>     [ line with SSL info here, if needed ]
>     Type "help" for help.
>
>     test=>
>
> I do feel that the help statement ought to be on its own line;
> the other way is going to look cluttered, particularly as soon
> as there's a version warning in there.

O.k. I am not trying to start an argument here but... I already sent 6
revisions of this patch that received comments and had thorough review
via Alvaro. I even took into account Tom's original comments from the
previous thread.

This much effort on something so simple makes it not worth the effort in
the first place.

Bruce with respect the only useful thing I have seen you do to the patch
in all this wrangling is realign the \? General options and frankly even
that is suspect in my opinion. Can we either throw it away and say,
"Nice try JD" or just commit the thing.

Joshua D. Drake


>
>             regards, tom lane
>


Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Joshua D. Drake wrote:
> O.k. I am not trying to start an argument here but... I already sent 6
> revisions of this patch that received comments and had thorough review
> via Alvaro. I even took into account Tom's original comments from the
> previous thread.
>
> This much effort on something so simple makes it not worth the effort in
> the first place.
>
> Bruce with respect the only useful thing I have seen you do to the patch
> in all this wrangling is realign the \? General options and frankly even
> that is suspect in my opinion. Can we either throw it away and say,
> "Nice try JD" or just commit the thing.

Your patch is getting the same review any other patch would have.  If
you want someone else to apply it I will stop working on it.

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

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

Re: Patch to change psql default banner v6

From
Andrew Dunstan
Date:

Joshua D. Drake wrote:
>
> O.k. I am not trying to start an argument here but... I already sent 6
> revisions of this patch that received comments and had thorough review
> via Alvaro. I even took into account Tom's original comments from the
> previous thread.
>
> This much effort on something so simple makes it not worth the effort
> in the first place.
>
>

Welcome to UI development. There is always *far* more argument of minor
matters of appearance than over anything else, in my experience.

cheers

andrew

Re: Patch to change psql default banner v6

From
"Joshua D. Drake"
Date:
On Thu, 15 May 2008 11:46:41 -0400 (EDT)
Bruce Momjian <bruce@momjian.us> wrote:

> > Bruce with respect the only useful thing I have seen you do to the
> > patch in all this wrangling is realign the \? General options and
> > frankly even that is suspect in my opinion. Can we either throw it
> > away and say, "Nice try JD" or just commit the thing.
>
> Your patch is getting the same review any other patch would have.  If
> you want someone else to apply it I will stop working on it.

I am not asking you to not review the patch. I am asking you to be
productive in doing so.  Your review of this patch is basically, "Even
though there were two very long threads with several (on the greater
side of several) different people contributing feedback, I think I know
better".

That behavior is frustrating, especially when I took an extreme amount
of effort to address all concerns ahead of the actual commit fest. I
wanted to make sure the patch would be easy to review and easy to
commit. If I thought I was going to have to have the argument all over
again, I just would have left the first submission as it was and then
we could have burned all the time during commit fest only.

Now Alvaro, Tom and I are all having the same discussion all over again
but this time with Bruce. It makes no sense.

Sincerely,

Joshua D. Drake

--
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



Attachment

Re: Patch to change psql default banner v6

From
Alvaro Herrera
Date:
Andrew Dunstan wrote:

> Welcome to UI development. There is always *far* more argument of minor
> matters of appearance than over anything else, in my experience.

Which is a good thing (in this case at least), because otherwise we
would end up with a crappy UI just because a single person thinks it's
"good enough".

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

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Oh, good point.  Let me look at that.  Thanks.  You prefer:
>
> >     $ sql test
> >     psql (8.4devel)
> >     Type "help" for help.
>
> >     test=> help
>
> Well, the question is still "where is the optional info going to go?"
>
> I think what I'd find nice looking is
>
>     $ psql test
>     psql 8.4devel  [ server version warning here, if needed ]
>     [ line with SSL info here, if needed ]
>     Type "help" for help.
>
>     test=>
>
> I do feel that the help statement ought to be on its own line;
> the other way is going to look cluttered, particularly as soon
> as there's a version warning in there.

OK, here is the normal startup now:

    $ sql test
    psql (8.4.0)
    Type "help" for help.

    test=>

Here is a minor version mismatch:

    $ sql test
    psql (8.4.0, server 8.4.1)
    Type "help" for help.

    test=>

Here is a major version mismatch:

    $ sql test
    psql (8.4.0, server 8.3.1)
            WARNING: psql version 8.4.0, server version 8.3.1.
            Some psql features might not work.
    Type "help" for help.

    test=>

I have also added '\g' to the 'help' display:

    test=> help

    You are using psql, the command-line interface to PostgreSQL.
            \? for psql help
            \h or \help for SQL help

            \g or ";" to execute a query
            \q to quit psql

            \copyright to view the copyright

    test=>

I don't know how to generate an SSL message.

With the new smaller \? "General" section, I though it was worth
considering if we still want to do the help banner the same.  Obviously
we do, but I wanted to explore it.

Patch attached.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/help.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.127
diff -c -c -r1.127 help.c
*** src/bin/psql/help.c    14 May 2008 15:30:22 -0000    1.127
--- src/bin/psql/help.c    15 May 2008 16:05:51 -0000
***************
*** 170,182 ****
       */
      fprintf(output, _("General\n"));
      fprintf(output, _("  \\copyright     show PostgreSQL usage and distribution terms\n"));
      fprintf(output, _("  \\h [NAME]      help on syntax of SQL commands, * for all commands\n"));
      fprintf(output, _("  \\q             quit psql\n"));
      fprintf(output, "\n");

      fprintf(output, _("Query Buffer\n"));
      fprintf(output, _("  \\e [FILE]      edit the query buffer (or file) with external editor\n"));
-     fprintf(output, _("  \\g [FILE]      send query buffer to server (and results to file or |pipe)\n"));
      fprintf(output, _("  \\p             show the contents of the query buffer\n"));
      fprintf(output, _("  \\r             reset (clear) the query buffer\n"));
  #ifdef USE_READLINE
--- 170,182 ----
       */
      fprintf(output, _("General\n"));
      fprintf(output, _("  \\copyright     show PostgreSQL usage and distribution terms\n"));
+     fprintf(output, _("  \\g [FILE] or ; execute query (and send results to file or |pipe)\n"));
      fprintf(output, _("  \\h [NAME]      help on syntax of SQL commands, * for all commands\n"));
      fprintf(output, _("  \\q             quit psql\n"));
      fprintf(output, "\n");

      fprintf(output, _("Query Buffer\n"));
      fprintf(output, _("  \\e [FILE]      edit the query buffer (or file) with external editor\n"));
      fprintf(output, _("  \\p             show the contents of the query buffer\n"));
      fprintf(output, _("  \\r             reset (clear) the query buffer\n"));
  #ifdef USE_READLINE
Index: src/bin/psql/mainloop.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v
retrieving revision 1.90
diff -c -c -r1.90 mainloop.c
*** src/bin/psql/mainloop.c    5 Apr 2008 03:40:15 -0000    1.90
--- src/bin/psql/mainloop.c    15 May 2008 16:05:51 -0000
***************
*** 177,186 ****
              (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
          {
              free(line);
!             puts(_("You are using psql, the command-line interface to PostgreSQL."));
!             puts(_("Enter SQL commands, or type \\? for a list of backslash options."));
!             puts(_("Use \\h for SQL command help."));
!             puts(_("Use \\q to quit."));
              fflush(stdout);
              continue;
          }
--- 177,189 ----
              (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
          {
              free(line);
!             puts(_("\nYou are using psql, the command-line interface to PostgreSQL."));
!             puts(_("\t\\? for psql help"));
!             puts(_("\t\\h or \\help for SQL help\n"));
!             puts(_("\t\\g or \";\" to execute a query"));
!             puts(_("\t\\q to quit psql\n"));
!             puts(_("\t\\copyright to view the copyright\n"));
!
              fflush(stdout);
              continue;
          }
Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.147
diff -c -c -r1.147 startup.c
*** src/bin/psql/startup.c    8 May 2008 17:04:26 -0000    1.147
--- src/bin/psql/startup.c    15 May 2008 16:05:52 -0000
***************
*** 317,342 ****
                      server_version = server_ver_str;
                  }

!                 printf(_("Welcome to %s %s (server %s), the PostgreSQL interactive terminal.\n\n"),
!                        pset.progname, PG_VERSION, server_version);
              }
              else
!                 printf(_("Welcome to %s %s, the PostgreSQL interactive terminal.\n\n"),
!                        pset.progname, PG_VERSION);
!
!             printf(_("Type:  \\copyright for distribution terms\n"
!                      "       \\h for help with SQL commands\n"
!                      "       \\? for help with psql commands\n"
!                   "       \\g or terminate with semicolon to execute query\n"
!                      "       \\q to quit\n\n"));

              if (pset.sversion / 100 != client_ver / 100)
!                 printf(_("WARNING:  You are connected to a server with major version %d.%d,\n"
!                          "but your %s client is major version %d.%d.  Some backslash commands,\n"
!                          "such as \\d, might not work properly.\n\n"),
!                        pset.sversion / 10000, (pset.sversion / 100) % 100,
!                        pset.progname,
!                        client_ver / 10000, (client_ver / 100) % 100);

  #ifdef USE_SSL
              printSSLInfo();
--- 317,335 ----
                      server_version = server_ver_str;
                  }

!                 printf(_("%s (%s, server %s)\n"),
!                 pset.progname, PG_VERSION, server_version);
              }
              else
!                 printf("%s (%s)\n", pset.progname, PG_VERSION);

              if (pset.sversion / 100 != client_ver / 100)
!                 printf(_("\tWARNING: %s version %d.%d, server version %d.%d.\n"
!                      "\tSome psql features might not work.\n"),
!                     pset.progname, client_ver / 10000, (client_ver / 100) % 100,
!                     pset.sversion / 10000, (pset.sversion / 100) % 100);
!
!             printf(_("Type \"help\" for help.\n\n"));

  #ifdef USE_SSL
              printSSLInfo();

Re: Patch to change psql default banner v6

From
Ron Mayer
Date:
Alvaro Herrera wrote:
> Andrew Dunstan wrote:
>
>> Welcome to UI development. There is always *far* more argument of minor
>> matters of appearance than over anything else, in my experience.
>
> Which is a good thing (in this case at least), because otherwise we
> would end up with a crappy UI just because a single person thinks it's
> "good enough".


This makes me think we shouldn't be hard-coding anything at all
as the welcome message; but rather having a default .psqlrc
in much the same way that that there's a default /etc/bash.bashrc
and /etc/csh.login.

Within that default .psqlrc we can put
    \qecho "Whatever the default message is"
or
    select "my message "+version();
to create the default, but then anyone with their own .psqlrc
can re-define it to whatever they think is a "good enough" UI.


Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Ron Mayer wrote:
> Alvaro Herrera wrote:
> > Andrew Dunstan wrote:
> >
> >> Welcome to UI development. There is always *far* more argument of minor
> >> matters of appearance than over anything else, in my experience.
> >
> > Which is a good thing (in this case at least), because otherwise we
> > would end up with a crappy UI just because a single person thinks it's
> > "good enough".
>
>
> This makes me think we shouldn't be hard-coding anything at all
> as the welcome message; but rather having a default .psqlrc
> in much the same way that that there's a default /etc/bash.bashrc
> and /etc/csh.login.
>
> Within that default .psqlrc we can put
>     \qecho "Whatever the default message is"
> or
>     select "my message "+version();
> to create the default, but then anyone with their own .psqlrc
> can re-define it to whatever they think is a "good enough" UI.

We could do that but we still have to design the default banner.

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

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

Re: Patch to change psql default banner v6

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Ron Mayer wrote:
>> This makes me think we shouldn't be hard-coding anything at all
>> as the welcome message; but rather having a default .psqlrc
>> in much the same way that that there's a default /etc/bash.bashrc
>> and /etc/csh.login.
>>
>> Within that default .psqlrc we can put
>> \qecho "Whatever the default message is"
>> or
>> select "my message "+version();
>> to create the default, but then anyone with their own .psqlrc
>> can re-define it to whatever they think is a "good enough" UI.

> We could do that but we still have to design the default banner.

More to the point, we would then have to design API with which a
custom .psqlrc could put out information about psql version,
server version, SSL status, etc.  It would take a lot of work
to make this approach actually useful, and there isn't demand
to justify it AFAIK.

It's worth polishing the default behavior in any case, because
that is what newbies are going to see.

            regards, tom lane

Re: Patch to change psql default banner v6

From
Alvaro Herrera
Date:
I'm OK with thisG but please move the printSSLInfo() call just before
echoing the help line.

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

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
>
> I'm OK with thisG but please move the printSSLInfo() call just before
> echoing the help line.

Oh, good catch, moved.  I also moved the Win32 code page message up too.
Patch attached.

I hacked up an example that shows both SSL and Win32 code page messages:

    $ psql test
    psql (8.4devel)
            SSL connection (cipher: 2343, bits: 512)
            WARNING: Console code page (323) differs from Windows code page (2323)
                     8-bit characters might not work correctly. See psql reference
                     page "Notes for Windows users" for details.
    Type "help" for help.

    test=>

With major version mismatches it looks like this:

    $ psql test
    psql (8.4devel)
            SSL connection (cipher: 2343, bits: 512)
            WARNING: Console code page (323) differs from Windows code page (2323)
                     8-bit characters might not work correctly. See psql reference
                     page "Notes for Windows users" for details.
        WARNING: psql version 8.4.0, server version 8.3.1.
             Some psql features might not work.
    Type "help" for help.

    test=>

By indenting those messages the 'help' message still stands out.
Adjustments?

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/help.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.127
diff -c -c -r1.127 help.c
*** src/bin/psql/help.c    14 May 2008 15:30:22 -0000    1.127
--- src/bin/psql/help.c    15 May 2008 19:17:27 -0000
***************
*** 170,182 ****
       */
      fprintf(output, _("General\n"));
      fprintf(output, _("  \\copyright     show PostgreSQL usage and distribution terms\n"));
      fprintf(output, _("  \\h [NAME]      help on syntax of SQL commands, * for all commands\n"));
      fprintf(output, _("  \\q             quit psql\n"));
      fprintf(output, "\n");

      fprintf(output, _("Query Buffer\n"));
      fprintf(output, _("  \\e [FILE]      edit the query buffer (or file) with external editor\n"));
-     fprintf(output, _("  \\g [FILE]      send query buffer to server (and results to file or |pipe)\n"));
      fprintf(output, _("  \\p             show the contents of the query buffer\n"));
      fprintf(output, _("  \\r             reset (clear) the query buffer\n"));
  #ifdef USE_READLINE
--- 170,182 ----
       */
      fprintf(output, _("General\n"));
      fprintf(output, _("  \\copyright     show PostgreSQL usage and distribution terms\n"));
+     fprintf(output, _("  \\g [FILE] or ; execute query (and send results to file or |pipe)\n"));
      fprintf(output, _("  \\h [NAME]      help on syntax of SQL commands, * for all commands\n"));
      fprintf(output, _("  \\q             quit psql\n"));
      fprintf(output, "\n");

      fprintf(output, _("Query Buffer\n"));
      fprintf(output, _("  \\e [FILE]      edit the query buffer (or file) with external editor\n"));
      fprintf(output, _("  \\p             show the contents of the query buffer\n"));
      fprintf(output, _("  \\r             reset (clear) the query buffer\n"));
  #ifdef USE_READLINE
Index: src/bin/psql/mainloop.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v
retrieving revision 1.90
diff -c -c -r1.90 mainloop.c
*** src/bin/psql/mainloop.c    5 Apr 2008 03:40:15 -0000    1.90
--- src/bin/psql/mainloop.c    15 May 2008 19:17:27 -0000
***************
*** 177,186 ****
              (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
          {
              free(line);
!             puts(_("You are using psql, the command-line interface to PostgreSQL."));
!             puts(_("Enter SQL commands, or type \\? for a list of backslash options."));
!             puts(_("Use \\h for SQL command help."));
!             puts(_("Use \\q to quit."));
              fflush(stdout);
              continue;
          }
--- 177,189 ----
              (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
          {
              free(line);
!             puts(_("\nYou are using psql, the command-line interface to PostgreSQL."));
!             puts(_("\t\\? for psql help"));
!             puts(_("\t\\h or \\help for SQL help\n"));
!             puts(_("\t\\g or \";\" to execute a query"));
!             puts(_("\t\\q to quit psql\n"));
!             puts(_("\t\\copyright to view the copyright\n"));
!
              fflush(stdout);
              continue;
          }
Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.147
diff -c -c -r1.147 startup.c
*** src/bin/psql/startup.c    8 May 2008 17:04:26 -0000    1.147
--- src/bin/psql/startup.c    15 May 2008 19:17:27 -0000
***************
*** 300,305 ****
--- 300,312 ----
          {
              int            client_ver = parse_version(PG_VERSION);

+ #ifdef USE_SSL
+             printSSLInfo();
+ #endif
+ #ifdef WIN32
+             checkWin32Codepage();
+ #endif
+
              if (pset.sversion != client_ver)
              {
                  const char *server_version;
***************
*** 317,349 ****
                      server_version = server_ver_str;
                  }

!                 printf(_("Welcome to %s %s (server %s), the PostgreSQL interactive terminal.\n\n"),
!                        pset.progname, PG_VERSION, server_version);
              }
              else
!                 printf(_("Welcome to %s %s, the PostgreSQL interactive terminal.\n\n"),
!                        pset.progname, PG_VERSION);
!
!             printf(_("Type:  \\copyright for distribution terms\n"
!                      "       \\h for help with SQL commands\n"
!                      "       \\? for help with psql commands\n"
!                   "       \\g or terminate with semicolon to execute query\n"
!                      "       \\q to quit\n\n"));

              if (pset.sversion / 100 != client_ver / 100)
!                 printf(_("WARNING:  You are connected to a server with major version %d.%d,\n"
!                          "but your %s client is major version %d.%d.  Some backslash commands,\n"
!                          "such as \\d, might not work properly.\n\n"),
!                        pset.sversion / 10000, (pset.sversion / 100) % 100,
!                        pset.progname,
!                        client_ver / 10000, (client_ver / 100) % 100);

! #ifdef USE_SSL
!             printSSLInfo();
! #endif
! #ifdef WIN32
!             checkWin32Codepage();
! #endif
          }

          if (!pset.notty)
--- 324,342 ----
                      server_version = server_ver_str;
                  }

!                 printf(_("%s (%s, server %s)\n"),
!                 pset.progname, PG_VERSION, server_version);
              }
              else
!                 printf("%s (%s)\n", pset.progname, PG_VERSION);

              if (pset.sversion / 100 != client_ver / 100)
!                 printf(_("\tWARNING: %s version %d.%d, server version %d.%d.\n"
!                      "\t         Some psql features might not work.\n"),
!                     pset.progname, client_ver / 10000, (client_ver / 100) % 100,
!                     pset.sversion / 10000, (pset.sversion / 100) % 100);

!             printf(_("Type \"help\" for help.\n\n"));
          }

          if (!pset.notty)
***************
*** 707,713 ****
          return;                    /* no SSL */

      SSL_get_cipher_bits(ssl, &sslbits);
!     printf(_("SSL connection (cipher: %s, bits: %i)\n\n"),
             SSL_get_cipher(ssl), sslbits);
  }
  #endif
--- 700,706 ----
          return;                    /* no SSL */

      SSL_get_cipher_bits(ssl, &sslbits);
!     printf(_("\tSSL connection (cipher: %s, bits: %i)\n"),
             SSL_get_cipher(ssl), sslbits);
  }
  #endif
***************
*** 729,737 ****
      concp = GetConsoleCP();
      if (wincp != concp)
      {
!         printf(_("Warning: Console code page (%u) differs from Windows code page (%u)\n"
!                  "         8-bit characters might not work correctly. See psql reference\n"
!                "         page \"Notes for Windows users\" for details.\n\n"),
                 concp, wincp);
      }
  }
--- 722,730 ----
      concp = GetConsoleCP();
      if (wincp != concp)
      {
!         printf(_("\tWARNING: Console code page (%u) differs from Windows code page (%u)\n"
!                  "\t         8-bit characters might not work correctly. See psql reference\n"
!                  "\t         page \"Notes for Windows users\" for details.\n"),
                 concp, wincp);
      }
  }

Re: Patch to change psql default banner v6

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> With major version mismatches it looks like this:

>     $ psql test
>     psql (8.4devel)
>             SSL connection (cipher: 2343, bits: 512)
>             WARNING: Console code page (323) differs from Windows code page (2323)
>                      8-bit characters might not work correctly. See psql reference
>                      page "Notes for Windows users" for details.
>         WARNING: psql version 8.4.0, server version 8.3.1.
>              Some psql features might not work.
>     Type "help" for help.

> By indenting those messages the 'help' message still stands out.

My advice: don't do that, it just looks weird.  Both of these look
fine to me:

$ psql test
psql (8.4devel)
SSL connection (cipher: 2343, bits: 512)
Type "help" for help.

$ psql test
psql (8.4devel)
SSL connection (cipher: 2343, bits: 512)
WARNING: Console code page (323) differs from Windows code page (2323)
         8-bit characters might not work correctly. See psql reference
         page "Notes for Windows users" for details.
WARNING: psql version 8.4.0, server version 8.3.1.
         Some psql features might not work.
Type "help" for help.

Also, maybe it's just me, but I think you have put the order of these
optional things exactly backwards.  I'd do

$ psql test
psql (8.4devel)
WARNING: psql version 8.4.0, server version 8.3.1.
         Some psql features might not work.
WARNING: Console code page (323) differs from Windows code page (2323)
         8-bit characters might not work correctly. See psql reference
         page "Notes for Windows users" for details.
SSL connection (cipher: 2343, bits: 512)
Type "help" for help.

Why?  Well, it's just more nearly the way it used to be.

            regards, tom lane

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > With major version mismatches it looks like this:
>
> >     $ psql test
> >     psql (8.4devel)
> >             SSL connection (cipher: 2343, bits: 512)
> >             WARNING: Console code page (323) differs from Windows code page (2323)
> >                      8-bit characters might not work correctly. See psql reference
> >                      page "Notes for Windows users" for details.
> >         WARNING: psql version 8.4.0, server version 8.3.1.
> >              Some psql features might not work.
> >     Type "help" for help.
>
> > By indenting those messages the 'help' message still stands out.
>
> My advice: don't do that, it just looks weird.  Both of these look
> fine to me:
>
> $ psql test
> psql (8.4devel)
> SSL connection (cipher: 2343, bits: 512)
> Type "help" for help.
>
> $ psql test
> psql (8.4devel)
> SSL connection (cipher: 2343, bits: 512)
> WARNING: Console code page (323) differs from Windows code page (2323)
>          8-bit characters might not work correctly. See psql reference
>          page "Notes for Windows users" for details.
> WARNING: psql version 8.4.0, server version 8.3.1.
>          Some psql features might not work.
> Type "help" for help.
>
> Also, maybe it's just me, but I think you have put the order of these
> optional things exactly backwards.  I'd do
>
> $ psql test
> psql (8.4devel)
> WARNING: psql version 8.4.0, server version 8.3.1.
>          Some psql features might not work.
> WARNING: Console code page (323) differs from Windows code page (2323)
>          8-bit characters might not work correctly. See psql reference
>          page "Notes for Windows users" for details.
> SSL connection (cipher: 2343, bits: 512)
> Type "help" for help.
>
> Why?  Well, it's just more nearly the way it used to be.

OK, here is the mega-print:

    $ psql test
    psql (8.4devel, server 8.4devel)
    WARNING: psql version 8.4, server version 8.4.
             Some psql features might not work.
    WARNING: Console code page (44) differs from Windows code page (55)
             8-bit characters might not work correctly. See psql reference
             page "Notes for Windows users" for details.
    SSL connection (cipher: 55, bits: 512)
    Type "help" for help.

    test=>

Patch at ftp://momjian.us/pub/postgresql/mypatches/help.

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

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

Re: Patch to change psql default banner v6

From
daveg
Date:
On Thu, May 15, 2008 at 10:20:53AM -0700, Ron Mayer wrote:
> Alvaro Herrera wrote:
> >Andrew Dunstan wrote:
> >
> >>Welcome to UI development. There is always *far* more argument of minor
> >>matters of appearance than over anything else, in my experience.
> >
> >Which is a good thing (in this case at least), because otherwise we
> >would end up with a crappy UI just because a single person thinks it's
> >"good enough".
>
>
> This makes me think we shouldn't be hard-coding anything at all
> as the welcome message; but rather having a default .psqlrc
> in much the same way that that there's a default /etc/bash.bashrc
> and /etc/csh.login.
>
> Within that default .psqlrc we can put
>    \qecho "Whatever the default message is"
> or
>    select "my message "+version();
> to create the default, but then anyone with their own .psqlrc
> can re-define it to whatever they think is a "good enough" UI.

+1
Including no banner at all please.


--
David Gould       daveg@sonic.net      510 536 1443    510 282 0869
If simplicity worked, the world would be overrun with insects.

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
daveg wrote:
> On Thu, May 15, 2008 at 10:20:53AM -0700, Ron Mayer wrote:
> > Alvaro Herrera wrote:
> > >Andrew Dunstan wrote:
> > >
> > >>Welcome to UI development. There is always *far* more argument of minor
> > >>matters of appearance than over anything else, in my experience.
> > >
> > >Which is a good thing (in this case at least), because otherwise we
> > >would end up with a crappy UI just because a single person thinks it's
> > >"good enough".
> >
> >
> > This makes me think we shouldn't be hard-coding anything at all
> > as the welcome message; but rather having a default .psqlrc
> > in much the same way that that there's a default /etc/bash.bashrc
> > and /etc/csh.login.
> >
> > Within that default .psqlrc we can put
> >    \qecho "Whatever the default message is"
> > or
> >    select "my message "+version();
> > to create the default, but then anyone with their own .psqlrc
> > can re-define it to whatever they think is a "good enough" UI.
>
> +1
> Including no banner at all please.

There is a psql quiet option that shows no banner:

       -q
       --quiet
              Specifies  that psql should do its work quietly. By
              default, it prints  welcome  messages  and  various
              informational  output. If this option is used, none
              of this happens. This is useful with the -c option.
              Within  psql you can also set the QUIET variable to
              achieve the same effect.

You could then use \echo in .psqlrc to make your own banner.

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

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

Re: Patch to change psql default banner v6

From
David Fetter
Date:
On Thu, May 15, 2008 at 12:09:25PM -0400, Alvaro Herrera wrote:
> Andrew Dunstan wrote:
>
> > Welcome to UI development. There is always *far* more argument of minor
> > matters of appearance than over anything else, in my experience.
>
> Which is a good thing (in this case at least), because otherwise we
> would end up with a crappy UI just because a single person thinks it's
> "good enough".

I hate to bike-shed this even further, but I'd like to make those
"incompatibility" messages just go away by making 8.4's psql (and all
those going forward) support every living version of Postgres at the
time of their release, so 8.4's psql would be able to talk seamlessly
to Postgres 7.4 :)

Cheers,
David (well, not really bike-shedding, but trying to propose a feature
that reduces the amount of UI clutter)
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Patch to change psql default banner v6

From
Gregory Stark
Date:
"Alvaro Herrera" <alvherre@commandprompt.com> writes:

> Bruce Momjian wrote:
>
>> I know we decided not to do that, but I am trying to figure out what the
>> goal if 'help' is?  To display the most frequently-used help commands?
>> Aren't they at the top of \?.
>
> The purpose of 'help' is to provide useful help.  If it only says "see \?"
> then it's just redirecting you somewhere else, which isn't useful.

Haven't been following this thread. Has the idea of making "help" just a
synonym for \? not come up? Or has it been rejected? It seems simplest.


--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Ask me about EnterpriseDB's On-Demand Production Tuning

Re: Patch to change psql default banner v6

From
Andrew Dunstan
Date:

David Fetter wrote:
>
> I hate to bike-shed this even further, but I'd like to make those
> "incompatibility" messages just go away by making 8.4's psql (and all
> those going forward) support every living version of Postgres at the
> time of their release, so 8.4's psql would be able to talk seamlessly
> to Postgres 7.4 :)
>
>

I think you must have been out in the sun too long.

Just look at the pg_dump code if you want something of an idea of what
this would involve.

cheers

andrew

Re: Patch to change psql default banner v6

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> David Fetter wrote:
>> I hate to bike-shed this even further, but I'd like to make those
>> "incompatibility" messages just go away by making 8.4's psql (and all
>> those going forward) support every living version of Postgres at the
>> time of their release,

> I think you must have been out in the sun too long.

Hey, he's welcome to try to do it.  But it's utterly unrelated to the
patch at hand, and we are not holding up the patch at hand until
something like that happens.

            regards, tom lane

Re: Patch to change psql default banner v6

From
David Fetter
Date:
On Thu, May 15, 2008 at 06:55:31PM -0400, Andrew Dunstan wrote:
> David Fetter wrote:
>>
>> I hate to bike-shed this even further, but I'd like to make those
>> "incompatibility" messages just go away by making 8.4's psql (and
>> all those going forward) support every living version of Postgres
>> at the time of their release, so 8.4's psql would be able to talk
>> seamlessly to Postgres 7.4 :)
>
> I think you must have been out in the sun too long.

One thing I really treasure about working on the Postgres project is
frank feedback. :)

> Just look at the pg_dump code if you want something of an idea of
> what this would involve.

Given that each previous version tied backslash commands to some
particular chunk of SQL, what would be the problem with either
immediately or lazily setting those to the chunks of SQL already
present in previous versions?

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Patch to change psql default banner v6

From
David Fetter
Date:
On Thu, May 15, 2008 at 06:57:12PM -0400, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > David Fetter wrote:
> >> I hate to bike-shed this even further, but I'd like to make those
> >> "incompatibility" messages just go away by making 8.4's psql (and
> >> all those going forward) support every living version of Postgres
> >> at the time of their release,
>
> > I think you must have been out in the sun too long.
>
> Hey, he's welcome to try to do it.  But it's utterly unrelated to
> the patch at hand, and we are not holding up the patch at hand until
> something like that happens.

Nor am I suggesting holding up this patch for that reason :)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Patch to change psql default banner v6

From
Andrew Dunstan
Date:

David Fetter wrote:
> On Thu, May 15, 2008 at 06:55:31PM -0400, Andrew Dunstan wrote:
>
>> David Fetter wrote:
>>
>>> I hate to bike-shed this even further, but I'd like to make those
>>> "incompatibility" messages just go away by making 8.4's psql (and
>>> all those going forward) support every living version of Postgres
>>> at the time of their release, so 8.4's psql would be able to talk
>>> seamlessly to Postgres 7.4 :)
>>>
>> I think you must have been out in the sun too long.
>>
>
> One thing I really treasure about working on the Postgres project is
> frank feedback. :)
>

I know you know me well enough to realise there was an implied smiley ;-)

>
>> Just look at the pg_dump code if you want something of an idea of
>> what this would involve.
>>
>
> Given that each previous version tied backslash commands to some
> particular chunk of SQL, what would be the problem with either
> immediately or lazily setting those to the chunks of SQL already
> present in previous versions?
>
>
>

First, this is not a cost free exercise - it increases code complexity
enormously.

Second, it's not nearly as easy as that:
. new commands have been added
. postgres features have been added
. catalogs have changed

Among other things, help and indeed the available command set would have
to become server version sensitive.

And you would greatly increase the bar for anyone wanting to add a new
command - now they (or someone) would have to work out how the command
would or might work n versions back, not just with the current dev version.

Doing it lazily isn't acceptable - if we promise \command compatibility
with previous server versions then we need to deliver it to the maximum
extent possible, and if we don't promise it there's no point in doing this.

And, as Tom says, it has nothing really to do with this patch.

cheers

andrew



Re: Patch to change psql default banner v6

From
Alvaro Herrera
Date:
Andrew Dunstan wrote:

> Second, it's not nearly as easy as that:
> . new commands have been added
> . postgres features have been added
> . catalogs have changed

Well, this just means a different piece of SQL needs to be sent for a
command depending on the server version, right?  It's not like that's
tremendously different.  The nice thing about most \X commands is that
they embed everything they need in a bunch of SQL, and they don't need
much else in C code.  So it's not all that difficult.

And for commands that have been added later, an initial version could
just say "this server version does not support this command".  It would
be already a huge improvement.

Probably the biggest change would be to support versions that did not
have schemas, but I think it would be OK to punt on that.  We already
stopped supporting 7.2 anyway.

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

Re: Patch to change psql default banner v6

From
Andrew Dunstan
Date:

Alvaro Herrera wrote:
> Andrew Dunstan wrote:
>
>
>> Second, it's not nearly as easy as that:
>> . new commands have been added
>> . postgres features have been added
>> . catalogs have changed
>>
>
> Well, this just means a different piece of SQL needs to be sent for a
> command depending on the server version, right?  It's not like that's
> tremendously different.  The nice thing about most \X commands is that
> they embed everything they need in a bunch of SQL, and they don't need
> much else in C code.  So it's not all that difficult.
>
> And for commands that have been added later, an initial version could
> just say "this server version does not support this command".  It would
> be already a huge improvement.
>
> Probably the biggest change would be to support versions that did not
> have schemas, but I think it would be OK to punt on that.  We already
> stopped supporting 7.2 anyway.
>

Have at it then. Prove me wrong.

cheers

andrew

Re: Patch to change psql default banner v6

From
Tom Lane
Date:
"Joshua D. Drake" <jd@commandprompt.com> writes:
> Andrew Dunstan wrote:
>> Have at it then. Prove me wrong.

> IMO the problem isn't the one off support for all supported version of
> Pg... say 7.4 -> 8.4. The problem is of on going maintenance.

> /me doesn't think it is worth the effort.

Since no one's done it yet, that would seem to be the consensus
opinion :-)

Still, it'd be interesting to see an attempt at making it go.
The costs of doing it now for existing versions would probably give us
a good idea of what the future maintenance effort might be like.
Without a real patch to look at, we're all just guessing about that.

            regards, tom lane

Re: Patch to change psql default banner v6

From
"Joshua D. Drake"
Date:
Andrew Dunstan wrote:

>> And for commands that have been added later, an initial version could
>> just say "this server version does not support this command".  It would
>> be already a huge improvement.
>>
>> Probably the biggest change would be to support versions that did not
>> have schemas, but I think it would be OK to punt on that.  We already
>> stopped supporting 7.2 anyway.
>>
>
> Have at it then. Prove me wrong.

IMO the problem isn't the one off support for all supported version of
Pg... say 7.4 -> 8.4. The problem is of on going maintenance.

/me doesn't think it is worth the effort.

Sincerely,

Joshua D. Drake



Re: Patch to change psql default banner v6

From
David Fetter
Date:
On Fri, May 16, 2008 at 01:22:55AM -0400, Tom Lane wrote:
> David Fetter <david@fetter.org> writes:
> > I believe there's a bug in this patch, namely that the warnings when
> > there's a server-client mismatch only appear at startup time.
>
> Please do not blame this patch for a problem that has been there all
> along.
>
> I don't say that the point doesn't need investigation, but blaming
> the patch-at-hand for the issue is just misleading.

The patch at hand, as you point out, emphasizes a problem that's been
there all along, namely that \c doesn't do the same things that
command line connection does.

I'm volunteering to make them use the same methods :)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Patch to change psql default banner v6

From
Tom Lane
Date:
David Fetter <david@fetter.org> writes:
> I believe there's a bug in this patch, namely that the warnings when
> there's a server-client mismatch only appear at startup time.

Please do not blame this patch for a problem that has been there all
along.

I don't say that the point doesn't need investigation, but blaming
the patch-at-hand for the issue is just misleading.

            regards, tom lane

Re: Patch to change psql default banner v6

From
David Fetter
Date:
On Thu, May 15, 2008 at 03:21:37PM -0400, Bruce Momjian wrote:
> Alvaro Herrera wrote:
> >
> > I'm OK with thisG but please move the printSSLInfo() call just before
> > echoing the help line.
>
> Oh, good catch, moved.  I also moved the Win32 code page message up too.
> Patch attached.
>
> I hacked up an example that shows both SSL and Win32 code page messages:

I believe there's a bug in this patch, namely that the warnings when
there's a server-client mismatch only appear at startup time.  This is
a pretty clear POLA violation, IMHO.

On my laptop, I have two pg instances running: 8.3.0 on port 5432, CVS
TIP on 2225.

Here's what I get if I invoke psql from the command line:

$ psql -p 5432 postgres
Welcome to psql 8.4devel (server 8.3.0), the PostgreSQL interactive terminal.

Type:  \copyright for distribution terms
       \h for help with SQL commands
       \? for help with psql commands
       \g or terminate with semicolon to execute query
       \q to quit

WARNING:  You are connected to a server with major version 8.3,
but your psql client is major version 8.4.  Some backslash commands,
such as \d, might not work properly.

Here's what I get if I use \c, having connected to CVS TIP first:

davidfetter@postgres=# \c - - - 5432
You are now connected to database "postgres" at port "5432".

I think that the warning should be consistently there on connect
instead of just at program start.

Not coincidentally, moving all the checks into one spot, i.e. making
startup.c and command.c call and test the same things to connect to a
database, advances my Evil Plan™ to make more interesting things
happen when switching versions :)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
Bruce Momjian wrote:
> OK, here is the mega-print:
>
>     $ psql test
>     psql (8.4devel, server 8.4devel)
>     WARNING: psql version 8.4, server version 8.4.
>              Some psql features might not work.
>     WARNING: Console code page (44) differs from Windows code page (55)
>              8-bit characters might not work correctly. See psql reference
>              page "Notes for Windows users" for details.
>     SSL connection (cipher: 55, bits: 512)
>     Type "help" for help.
>
>     test=>
>

Updated patch applied, docs adjusted for new psql startup banner.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/start.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/start.sgml,v
retrieving revision 1.46
diff -c -c -r1.46 start.sgml
*** doc/src/sgml/start.sgml    23 Jan 2008 02:04:47 -0000    1.46
--- doc/src/sgml/start.sgml    16 May 2008 17:06:38 -0000
***************
*** 329,341 ****
      In <command>psql</command>, you will be greeted with the following
      message:
  <screen>
! Welcome to psql &version;, the PostgreSQL interactive terminal.
!
! Type:  \copyright for distribution terms
!        \h for help with SQL commands
!        \? for help with psql commands
!        \g or terminate with semicolon to execute query
!        \q to quit

  mydb=>
  </screen>
--- 329,336 ----
      In <command>psql</command>, you will be greeted with the following
      message:
  <screen>
! psql (&version;)
! Type "help" for help.

  mydb=>
  </screen>
Index: doc/src/sgml/ref/psql-ref.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v
retrieving revision 1.205
diff -c -c -r1.205 psql-ref.sgml
*** doc/src/sgml/ref/psql-ref.sgml    16 May 2008 16:59:05 -0000    1.205
--- doc/src/sgml/ref/psql-ref.sgml    16 May 2008 17:06:38 -0000
***************
*** 571,583 ****
      the string <literal>=></literal>. For example:
  <programlisting>
  $ <userinput>psql testdb</userinput>
! Welcome to psql &version;, the PostgreSQL interactive terminal.

! Type:  \copyright for distribution terms
!        \h for help with SQL commands
!        \? for help with psql commands
!        \g or terminate with semicolon to execute query
!        \q to quit

  testdb=>
  </programlisting>
--- 571,580 ----
      the string <literal>=></literal>. For example:
  <programlisting>
  $ <userinput>psql testdb</userinput>
! psql (&version;)
! Type "help" for help.

! test=>

  testdb=>
  </programlisting>
Index: src/bin/psql/help.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.127
diff -c -c -r1.127 help.c
*** src/bin/psql/help.c    14 May 2008 15:30:22 -0000    1.127
--- src/bin/psql/help.c    16 May 2008 17:06:39 -0000
***************
*** 170,182 ****
       */
      fprintf(output, _("General\n"));
      fprintf(output, _("  \\copyright     show PostgreSQL usage and distribution terms\n"));
      fprintf(output, _("  \\h [NAME]      help on syntax of SQL commands, * for all commands\n"));
      fprintf(output, _("  \\q             quit psql\n"));
      fprintf(output, "\n");

      fprintf(output, _("Query Buffer\n"));
      fprintf(output, _("  \\e [FILE]      edit the query buffer (or file) with external editor\n"));
-     fprintf(output, _("  \\g [FILE]      send query buffer to server (and results to file or |pipe)\n"));
      fprintf(output, _("  \\p             show the contents of the query buffer\n"));
      fprintf(output, _("  \\r             reset (clear) the query buffer\n"));
  #ifdef USE_READLINE
--- 170,182 ----
       */
      fprintf(output, _("General\n"));
      fprintf(output, _("  \\copyright     show PostgreSQL usage and distribution terms\n"));
+     fprintf(output, _("  \\g [FILE] or ; execute query (and send results to file or |pipe)\n"));
      fprintf(output, _("  \\h [NAME]      help on syntax of SQL commands, * for all commands\n"));
      fprintf(output, _("  \\q             quit psql\n"));
      fprintf(output, "\n");

      fprintf(output, _("Query Buffer\n"));
      fprintf(output, _("  \\e [FILE]      edit the query buffer (or file) with external editor\n"));
      fprintf(output, _("  \\p             show the contents of the query buffer\n"));
      fprintf(output, _("  \\r             reset (clear) the query buffer\n"));
  #ifdef USE_READLINE
Index: src/bin/psql/mainloop.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v
retrieving revision 1.90
diff -c -c -r1.90 mainloop.c
*** src/bin/psql/mainloop.c    5 Apr 2008 03:40:15 -0000    1.90
--- src/bin/psql/mainloop.c    16 May 2008 17:06:39 -0000
***************
*** 177,186 ****
              (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
          {
              free(line);
!             puts(_("You are using psql, the command-line interface to PostgreSQL."));
!             puts(_("Enter SQL commands, or type \\? for a list of backslash options."));
!             puts(_("Use \\h for SQL command help."));
!             puts(_("Use \\q to quit."));
              fflush(stdout);
              continue;
          }
--- 177,189 ----
              (line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
          {
              free(line);
!             puts(_("\nYou are using psql, the command-line interface to PostgreSQL."));
!             puts(_("\t\\? for psql help"));
!             puts(_("\t\\h or \\help for SQL help\n"));
!             puts(_("\t\\g or \";\" to execute a query"));
!             puts(_("\t\\q to quit psql\n"));
!             puts(_("\t\\copyright to view the copyright\n"));
!
              fflush(stdout);
              continue;
          }
Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.147
diff -c -c -r1.147 startup.c
*** src/bin/psql/startup.c    8 May 2008 17:04:26 -0000    1.147
--- src/bin/psql/startup.c    16 May 2008 17:06:39 -0000
***************
*** 317,349 ****
                      server_version = server_ver_str;
                  }

!                 printf(_("Welcome to %s %s (server %s), the PostgreSQL interactive terminal.\n\n"),
!                        pset.progname, PG_VERSION, server_version);
              }
              else
!                 printf(_("Welcome to %s %s, the PostgreSQL interactive terminal.\n\n"),
!                        pset.progname, PG_VERSION);
!
!             printf(_("Type:  \\copyright for distribution terms\n"
!                      "       \\h for help with SQL commands\n"
!                      "       \\? for help with psql commands\n"
!                   "       \\g or terminate with semicolon to execute query\n"
!                      "       \\q to quit\n\n"));

              if (pset.sversion / 100 != client_ver / 100)
!                 printf(_("WARNING:  You are connected to a server with major version %d.%d,\n"
!                          "but your %s client is major version %d.%d.  Some backslash commands,\n"
!                          "such as \\d, might not work properly.\n\n"),
!                        pset.sversion / 10000, (pset.sversion / 100) % 100,
!                        pset.progname,
!                        client_ver / 10000, (client_ver / 100) % 100);

- #ifdef USE_SSL
-             printSSLInfo();
- #endif
  #ifdef WIN32
              checkWin32Codepage();
  #endif
          }

          if (!pset.notty)
--- 317,342 ----
                      server_version = server_ver_str;
                  }

!                 printf(_("%s (%s, server %s)\n"),
!                 pset.progname, PG_VERSION, server_version);
              }
              else
!                 printf("%s (%s)\n", pset.progname, PG_VERSION);

              if (pset.sversion / 100 != client_ver / 100)
!                 printf(_("WARNING: %s version %d.%d, server version %d.%d.\n"
!                      "         Some psql features might not work.\n"),
!                     pset.progname, client_ver / 10000, (client_ver / 100) % 100,
!                     pset.sversion / 10000, (pset.sversion / 100) % 100);

  #ifdef WIN32
              checkWin32Codepage();
  #endif
+ #ifdef USE_SSL
+             printSSLInfo();
+ #endif
+
+             printf(_("Type \"help\" for help.\n\n"));
          }

          if (!pset.notty)
***************
*** 707,713 ****
          return;                    /* no SSL */

      SSL_get_cipher_bits(ssl, &sslbits);
!     printf(_("SSL connection (cipher: %s, bits: %i)\n\n"),
             SSL_get_cipher(ssl), sslbits);
  }
  #endif
--- 700,706 ----
          return;                    /* no SSL */

      SSL_get_cipher_bits(ssl, &sslbits);
!     printf(_("SSL connection (cipher: %s, bits: %i)\n"),
             SSL_get_cipher(ssl), sslbits);
  }
  #endif
***************
*** 729,737 ****
      concp = GetConsoleCP();
      if (wincp != concp)
      {
!         printf(_("Warning: Console code page (%u) differs from Windows code page (%u)\n"
                   "         8-bit characters might not work correctly. See psql reference\n"
!                "         page \"Notes for Windows users\" for details.\n\n"),
                 concp, wincp);
      }
  }
--- 722,730 ----
      concp = GetConsoleCP();
      if (wincp != concp)
      {
!         printf(_("WARNING: Console code page (%u) differs from Windows code page (%u)\n"
                   "         8-bit characters might not work correctly. See psql reference\n"
!                  "         page \"Notes for Windows users\" for details.\n"),
                 concp, wincp);
      }
  }

Re: Patch to change psql default banner v6

From
Guillaume Lelarge
Date:
Tom Lane a écrit :
> "Joshua D. Drake" <jd@commandprompt.com> writes:
>> Andrew Dunstan wrote:
>>> Have at it then. Prove me wrong.
>
>> IMO the problem isn't the one off support for all supported version of
>> Pg... say 7.4 -> 8.4. The problem is of on going maintenance.
>
>> /me doesn't think it is worth the effort.
>
> Since no one's done it yet, that would seem to be the consensus
> opinion :-)
>
> Still, it'd be interesting to see an attempt at making it go.
> The costs of doing it now for existing versions would probably give us
> a good idea of what the future maintenance effort might be like.
> Without a real patch to look at, we're all just guessing about that.
>

Here is a patch that tries to implement this. Meta-commands should work
from 7.4 to 8.4-devel releases. It was not hard to do, and I don't think
it really is a burden to maintain.

One part left to fix is \du and \dg commands. I would be glad to
continue to work on this but I would prefer to have comments before.

Thanks.

Regards.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
Index: describe.c
===================================================================
RCS file: /opt/cvsroot_postgresql/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.173
diff -c -c -r1.173 describe.c
*** describe.c    13 May 2008 00:23:17 -0000    1.173
--- describe.c    19 May 2008 17:19:28 -0000
***************
*** 59,68 ****
       * There are two kinds of aggregates: ones that work on particular types
       * and ones that work on all (denoted by input type = "any")
       */
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname as \"%s\",\n"
                        "  p.proname AS \"%s\",\n"
!                   "  pg_catalog.format_type(p.prorettype, NULL) AS \"%s\",\n"
                        "  CASE WHEN p.pronargs = 0\n"
                        "    THEN CAST('*' AS pg_catalog.text)\n"
                        "    ELSE\n"
--- 59,75 ----
       * There are two kinds of aggregates: ones that work on particular types
       * and ones that work on all (denoted by input type = "any")
       */
+
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname as \"%s\",\n"
                        "  p.proname AS \"%s\",\n"
!                       "  pg_catalog.format_type(p.prorettype, NULL) AS \"%s\"",
!                       gettext_noop("Schema"),
!                       gettext_noop("Name"),
!                       gettext_noop("Result data type"));
!
!     if (pset.sversion >= 80100)
!         appendPQExpBuffer(&buf, ",\n"
                        "  CASE WHEN p.pronargs = 0\n"
                        "    THEN CAST('*' AS pg_catalog.text)\n"
                        "    ELSE\n"
***************
*** 72,86 ****
                        "      FROM\n"
                        "        pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n"
                        "    ), ', ')\n"
!                       "  END AS \"%s\",\n"
!                  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"\n"
                        "FROM pg_catalog.pg_proc p\n"
         "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n"
                        "WHERE p.proisagg\n",
-                       gettext_noop("Schema"),
-                       gettext_noop("Name"),
-                       gettext_noop("Result data type"),
-                       gettext_noop("Argument data types"),
                        gettext_noop("Description"));

      processSQLNamePattern(pset.db, &buf, pattern, true, false,
--- 79,91 ----
                        "      FROM\n"
                        "        pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n"
                        "    ), ', ')\n"
!                       "  END AS \"%s\"",
!                       gettext_noop("Argument data types"));
!
!     appendPQExpBuffer(&buf, ",\n  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"\n"
                        "FROM pg_catalog.pg_proc p\n"
         "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n"
                        "WHERE p.proisagg\n",
                        gettext_noop("Description"));

      processSQLNamePattern(pset.db, &buf, pattern, true, false,
***************
*** 132,142 ****
                        gettext_noop("Location"));

      if (verbose)
          appendPQExpBuffer(&buf,
!                           ",\n  spcacl AS \"%s\""
!          ",\n  pg_catalog.shobj_description(oid, 'pg_tablespace') AS \"%s\"",
!                           gettext_noop("Access privileges"),
                            gettext_noop("Description"));

      appendPQExpBuffer(&buf,
                        "\nFROM pg_catalog.pg_tablespace\n");
--- 137,151 ----
                        gettext_noop("Location"));

      if (verbose)
+     {
          appendPQExpBuffer(&buf,
!                           ",\n  spcacl AS \"%s\"",
!                           gettext_noop("Access privileges"));
!         if (pset.sversion >= 80200)
!             appendPQExpBuffer(&buf,
!                           ",\n  pg_catalog.shobj_description(oid, 'pg_tablespace') AS \"%s\"",
                            gettext_noop("Description"));
+     }

      appendPQExpBuffer(&buf,
                        "\nFROM pg_catalog.pg_tablespace\n");
***************
*** 179,186 ****
                        "SELECT n.nspname as \"%s\",\n"
                        "  p.proname as \"%s\",\n"
                        "  CASE WHEN p.proretset THEN 'setof ' ELSE '' END ||\n"
!                   "  pg_catalog.format_type(p.prorettype, NULL) as \"%s\",\n"
!                       "  CASE WHEN proallargtypes IS NOT NULL THEN\n"
                        "    pg_catalog.array_to_string(ARRAY(\n"
                        "      SELECT\n"
                        "        CASE\n"
--- 188,201 ----
                        "SELECT n.nspname as \"%s\",\n"
                        "  p.proname as \"%s\",\n"
                        "  CASE WHEN p.proretset THEN 'setof ' ELSE '' END ||\n"
!                   "  pg_catalog.format_type(p.prorettype, NULL) as \"%s\"",
!                       gettext_noop("Schema"),
!                       gettext_noop("Name"),
!                       gettext_noop("Result data type"));
!
!     if (pset.sversion >= 80100)
!         appendPQExpBuffer(&buf,
!                       ",\n  CASE WHEN proallargtypes IS NOT NULL THEN\n"
                        "    pg_catalog.array_to_string(ARRAY(\n"
                        "      SELECT\n"
                        "        CASE\n"
***************
*** 208,216 ****
                        "        pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n"
                        "    ), ', ')\n"
                        "  END AS \"%s\"",
-                       gettext_noop("Schema"),
-                       gettext_noop("Name"),
-                       gettext_noop("Result data type"),
                        gettext_noop("Argument data types"));

      if (verbose)
--- 223,228 ----
***************
*** 220,226 ****
                            "  WHEN p.provolatile = 's' THEN 'stable'\n"
                            "  WHEN p.provolatile = 'v' THEN 'volatile'\n"
                            "END as \"%s\""
!                           ",\n  r.rolname as \"%s\",\n"
                            "  l.lanname as \"%s\",\n"
                            "  p.prosrc as \"%s\",\n"
                    "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
--- 232,238 ----
                            "  WHEN p.provolatile = 's' THEN 'stable'\n"
                            "  WHEN p.provolatile = 'v' THEN 'volatile'\n"
                            "END as \"%s\""
!                           ",\n  pg_catalog.pg_get_userbyid(proowner) as \"%s\",\n"
                            "  l.lanname as \"%s\",\n"
                            "  p.prosrc as \"%s\",\n"
                    "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
***************
*** 238,245 ****
          appendPQExpBuffer(&buf,
                            "\nFROM pg_catalog.pg_proc p"
          "\n     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace"
!              "\n     LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang"
!                 "\n     JOIN pg_catalog.pg_roles r ON r.oid = p.proowner\n");

      /*
       * we skip in/out funcs by excluding functions that take or return cstring
--- 250,256 ----
          appendPQExpBuffer(&buf,
                            "\nFROM pg_catalog.pg_proc p"
          "\n     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace"
!              "\n     LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang\n");

      /*
       * we skip in/out funcs by excluding functions that take or return cstring
***************
*** 254,260 ****
                            "n.nspname", "p.proname", NULL,
                            "pg_catalog.pg_function_is_visible(p.oid)");

!     appendPQExpBuffer(&buf, "ORDER BY 1, 2, 3, 4;");

      res = PSQLexec(buf.data, false);
      termPQExpBuffer(&buf);
--- 265,273 ----
                            "n.nspname", "p.proname", NULL,
                            "pg_catalog.pg_function_is_visible(p.oid)");

!     appendPQExpBuffer(&buf, "ORDER BY 1, 2, 3");
!     if (pset.sversion >= 80100)
!         appendPQExpBuffer(&buf, ", 4;");

      res = PSQLexec(buf.data, false);
      termPQExpBuffer(&buf);
***************
*** 292,298 ****
                        gettext_noop("Schema"),
                        gettext_noop("Name"));
      if (verbose)
!         appendPQExpBuffer(&buf,
                            "  t.typname AS \"%s\",\n"
                            "  CASE WHEN t.typrelid != 0\n"
                            "      THEN CAST('tuple' AS pg_catalog.text)\n"
--- 305,327 ----
                        gettext_noop("Schema"),
                        gettext_noop("Name"));
      if (verbose)
!     {
!         if (pset.sversion < 80300)
!         {
!             appendPQExpBuffer(&buf,
!                           "  t.typname AS \"%s\",\n"
!                           "  CASE WHEN t.typrelid != 0\n"
!                           "      THEN CAST('tuple' AS pg_catalog.text)\n"
!                           "    WHEN t.typlen < 0\n"
!                           "      THEN CAST('var' AS pg_catalog.text)\n"
!                           "    ELSE CAST(t.typlen AS pg_catalog.text)\n"
!                           "  END AS \"%s\",\n",
!                           gettext_noop("Internal name"),
!                           gettext_noop("Size"));
!         }
!         else
!         {
!             appendPQExpBuffer(&buf,
                            "  t.typname AS \"%s\",\n"
                            "  CASE WHEN t.typrelid != 0\n"
                            "      THEN CAST('tuple' AS pg_catalog.text)\n"
***************
*** 312,317 ****
--- 341,348 ----
                            gettext_noop("Internal name"),
                            gettext_noop("Size"),
                            gettext_noop("Elements"));
+         }
+     }

      appendPQExpBuffer(&buf,
                        "  pg_catalog.obj_description(t.oid, 'pg_type') as \"%s\"\n",
***************
*** 419,425 ****

      printfPQExpBuffer(&buf,
                        "SELECT d.datname as \"%s\",\n"
!                       "       r.rolname as \"%s\",\n"
                        "       pg_catalog.pg_encoding_to_char(d.encoding) as \"%s\",\n"
                        "       d.datacl as \"%s\"",
                        gettext_noop("Name"),
--- 450,456 ----

      printfPQExpBuffer(&buf,
                        "SELECT d.datname as \"%s\",\n"
!                       "       pg_catalog.pg_get_userbyid(d.datdba) as \"%s\",\n"
                        "       pg_catalog.pg_encoding_to_char(d.encoding) as \"%s\",\n"
                        "       d.datacl as \"%s\"",
                        gettext_noop("Name"),
***************
*** 428,450 ****
                        gettext_noop("Access Privileges"));
      if (verbose)
      {
!         appendPQExpBuffer(&buf,
                            ",\n       CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
                            "            THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
                            "            ELSE 'No Access'\n"
                            "       END as \"%s\"",
                            gettext_noop("Size"));
!         appendPQExpBuffer(&buf,
                            ",\n       t.spcname as \"%s\"",
                            gettext_noop("Tablespace"));
!         appendPQExpBuffer(&buf,
                            ",\n       pg_catalog.shobj_description(d.oid, 'pg_database') as \"%s\"",
                            gettext_noop("Description"));
      }
      appendPQExpBuffer(&buf,
!                       "\nFROM pg_catalog.pg_database d"
!                       "\n  JOIN pg_catalog.pg_roles r ON d.datdba = r.oid\n");
!     if (verbose)
          appendPQExpBuffer(&buf,
             "  JOIN pg_catalog.pg_tablespace t on d.dattablespace = t.oid\n");
      appendPQExpBuffer(&buf, "ORDER BY 1;");
--- 459,483 ----
                        gettext_noop("Access Privileges"));
      if (verbose)
      {
!         if (pset.sversion >= 80200)
!             appendPQExpBuffer(&buf,
                            ",\n       CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
                            "            THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
                            "            ELSE 'No Access'\n"
                            "       END as \"%s\"",
                            gettext_noop("Size"));
!         if (pset.sversion >= 80100)
!             appendPQExpBuffer(&buf,
                            ",\n       t.spcname as \"%s\"",
                            gettext_noop("Tablespace"));
!         if (pset.sversion >= 80200)
!             appendPQExpBuffer(&buf,
                            ",\n       pg_catalog.shobj_description(d.oid, 'pg_database') as \"%s\"",
                            gettext_noop("Description"));
      }
      appendPQExpBuffer(&buf,
!                       "\nFROM pg_catalog.pg_database d\n");
!     if (verbose && pset.sversion >= 80100)
          appendPQExpBuffer(&buf,
             "  JOIN pg_catalog.pg_tablespace t on d.dattablespace = t.oid\n");
      appendPQExpBuffer(&buf, "ORDER BY 1;");
***************
*** 484,499 ****
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname as \"%s\",\n"
                        "  c.relname as \"%s\",\n"
!                       "  CASE c.relkind WHEN 'r' THEN '%s' WHEN 'v' THEN '%s' WHEN 'S' THEN '%s' END as \"%s\",\n"
!                       "  pg_catalog.array_to_string(c.relacl, E'\\n') as \"%s\"\n"
!                       "FROM pg_catalog.pg_class c\n"
!        "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
!                       "WHERE c.relkind IN ('r', 'v', 'S')\n",
                        gettext_noop("Schema"),
                        gettext_noop("Name"),
                        gettext_noop("table"), gettext_noop("view"), gettext_noop("sequence"),
!                       gettext_noop("Type"),
                        gettext_noop("Access privileges"));

      /*
       * Unless a schema pattern is specified, we suppress system and temp
--- 517,538 ----
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname as \"%s\",\n"
                        "  c.relname as \"%s\",\n"
!                       "  CASE c.relkind WHEN 'r' THEN '%s' WHEN 'v' THEN '%s' WHEN 'S' THEN '%s' END as \"%s\",\n",
                        gettext_noop("Schema"),
                        gettext_noop("Name"),
                        gettext_noop("table"), gettext_noop("view"), gettext_noop("sequence"),
!                       gettext_noop("Type"));
!
!     if (pset.sversion >= 80100)
!         appendPQExpBuffer(&buf, "  pg_catalog.array_to_string(c.relacl, E'\\n') as \"%s\"\n",
!                       gettext_noop("Access privileges"));
!     else
!         appendPQExpBuffer(&buf, "  pg_catalog.array_to_string(c.relacl, '\n') as \"%s\"\n",
                        gettext_noop("Access privileges"));
+
+     appendPQExpBuffer(&buf, "FROM pg_catalog.pg_class c\n"
+        "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
+                       "WHERE c.relkind IN ('r', 'v', 'S')\n");

      /*
       * Unless a schema pattern is specified, we suppress system and temp
***************
*** 1572,1578 ****

      initPQExpBuffer(&buf);

!     appendPQExpBufferStr(&buf,
                           "SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
                           "  r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
                           "  r.rolconnlimit,\n"
--- 1611,1630 ----

      initPQExpBuffer(&buf);

!     if (pset.sversion < 80100)
!     {
!         appendPQExpBufferStr(&buf,
!                       "SELECT u.usename AS rolname,\n"
!                       "  u.usesuper AS rolsuper,\n"
!                       "  FALSE AS rolinherit, FALSE AS rolcreaterole,\n"
!                       "  usecreatedb AS rolcreatedb, TRUE AS rolcanlogin,\n"
!                       "  -1 AS rolconnlimit,\n"
!                       "  ARRAY(SELECT g.groname FROM pg_catalog.pg_group g WHERE u.usesysid = ANY(g.grolist)) as
memberof\n"
!                       "FROM pg_catalog.pg_user u\n");
!     }
!     else
!     {
!         appendPQExpBufferStr(&buf,
                           "SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
                           "  r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
                           "  r.rolconnlimit,\n"
***************
*** 1581,1593 ****
                           "        JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
                           "        WHERE m.member = r.oid) as memberof");

!     if (verbose)
!     {
!         appendPQExpBufferStr(&buf, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
!         ncols++;
!     }

!     appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_roles r\n");

      processSQLNamePattern(pset.db, &buf, pattern, false, false,
                            NULL, "r.rolname", NULL, NULL);
--- 1633,1646 ----
                           "        JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
                           "        WHERE m.member = r.oid) as memberof");

!         if (pset.sversion >= 80200 && verbose)
!         {
!             appendPQExpBufferStr(&buf, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
!             ncols++;
!         }

!         appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_roles r\n");
!     }

      processSQLNamePattern(pset.db, &buf, pattern, false, false,
                            NULL, "r.rolname", NULL, NULL);
***************
*** 1607,1613 ****
      printTableAddHeader(&cont, gettext_noop("Attributes"), true, align);
      printTableAddHeader(&cont, gettext_noop("Member of"), true, align);

!     if (verbose)
          printTableAddHeader(&cont, gettext_noop("Description"), true, align);

      for (i = 0; i < nrows; i++)
--- 1660,1666 ----
      printTableAddHeader(&cont, gettext_noop("Attributes"), true, align);
      printTableAddHeader(&cont, gettext_noop("Member of"), true, align);

!     if (pset.sversion >= 80200 && verbose)
          printTableAddHeader(&cont, gettext_noop("Description"), true, align);

      for (i = 0; i < nrows; i++)
***************
*** 1650,1656 ****

          printTableAddCell(&cont, PQgetvalue(res, i, 7), false);

!         if (verbose)
              printTableAddCell(&cont, PQgetvalue(res, i, 8), false);
      }
      termPQExpBuffer(&buf);
--- 1703,1709 ----

          printTableAddCell(&cont, PQgetvalue(res, i, 7), false);

!         if (pset.sversion >= 80200 && verbose)
              printTableAddCell(&cont, PQgetvalue(res, i, 8), false);
      }
      termPQExpBuffer(&buf);
***************
*** 1716,1722 ****
                        "SELECT n.nspname as \"%s\",\n"
                        "  c.relname as \"%s\",\n"
                        "  CASE c.relkind WHEN 'r' THEN '%s' WHEN 'v' THEN '%s' WHEN 'i' THEN '%s' WHEN 'S' THEN '%s'
WHEN's' THEN '%s' END as \"%s\",\n" 
!                       "  r.rolname as \"%s\"",
                        gettext_noop("Schema"),
                        gettext_noop("Name"),
                        gettext_noop("table"), gettext_noop("view"), gettext_noop("index"), gettext_noop("sequence"),
gettext_noop("special"),
--- 1769,1775 ----
                        "SELECT n.nspname as \"%s\",\n"
                        "  c.relname as \"%s\",\n"
                        "  CASE c.relkind WHEN 'r' THEN '%s' WHEN 'v' THEN '%s' WHEN 'i' THEN '%s' WHEN 'S' THEN '%s'
WHEN's' THEN '%s' END as \"%s\",\n" 
!                       "  pg_catalog.pg_get_userbyid(c.relowner) as \"%s\"",
                        gettext_noop("Schema"),
                        gettext_noop("Name"),
                        gettext_noop("table"), gettext_noop("view"), gettext_noop("index"), gettext_noop("sequence"),
gettext_noop("special"),
***************
*** 1730,1738 ****

      if (verbose)
      {
!         appendPQExpBuffer(&buf,
                            ",\n  pg_catalog.pg_size_pretty(pg_catalog.pg_relation_size(c.oid)) as \"%s\"",
                              gettext_noop("Size"));
          appendPQExpBuffer(&buf,
                ",\n  pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
                            gettext_noop("Description"));
--- 1783,1794 ----

      if (verbose)
      {
!         if (pset.sversion >= 80100)
!         {
!             appendPQExpBuffer(&buf,
                            ",\n  pg_catalog.pg_size_pretty(pg_catalog.pg_relation_size(c.oid)) as \"%s\"",
                              gettext_noop("Size"));
+         }
          appendPQExpBuffer(&buf,
                ",\n  pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
                            gettext_noop("Description"));
***************
*** 1740,1746 ****

      appendPQExpBuffer(&buf,
                        "\nFROM pg_catalog.pg_class c"
-                     "\n     JOIN pg_catalog.pg_roles r ON r.oid = c.relowner"
       "\n     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
      if (showIndexes)
          appendPQExpBuffer(&buf,
--- 1796,1801 ----
***************
*** 1985,1991 ****
      initPQExpBuffer(&buf);
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname AS \"%s\",\n"
!                       "       r.rolname AS \"%s\"",
                        gettext_noop("Name"),
                        gettext_noop("Owner"));

--- 2040,2046 ----
      initPQExpBuffer(&buf);
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname AS \"%s\",\n"
!                       "       pg_catalog.pg_get_userbyid(nspowner) AS \"%s\"",
                        gettext_noop("Name"),
                        gettext_noop("Owner"));

***************
*** 1997,2004 ****
                            gettext_noop("Description"));

      appendPQExpBuffer(&buf,
!               "\nFROM pg_catalog.pg_namespace n JOIN pg_catalog.pg_roles r\n"
!                       "       ON n.nspowner=r.oid\n"
                        "WHERE    (n.nspname !~ '^pg_temp_' OR\n"
             "         n.nspname = (pg_catalog.current_schemas(true))[1])\n");        /* temp schema is first */

--- 2052,2058 ----
                            gettext_noop("Description"));

      appendPQExpBuffer(&buf,
!               "\nFROM pg_catalog.pg_namespace n\n"
                        "WHERE    (n.nspname !~ '^pg_temp_' OR\n"
             "         n.nspname = (pg_catalog.current_schemas(true))[1])\n");        /* temp schema is first */

***************
*** 2035,2040 ****
--- 2089,2101 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text parsers.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      if (verbose)
          return listTSParsersVerbose(pattern);

***************
*** 2083,2088 ****
--- 2144,2156 ----
      PGresult   *res;
      int            i;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text parser.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,
***************
*** 2150,2155 ****
--- 2218,2230 ----
      printQueryOpt myopt = pset.popt;
      static const bool trans_columns[] = {true, false, false};

+     if (pset.sversion < 80000)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text parser.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,
***************
*** 2261,2266 ****
--- 2336,2348 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text dictionary.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,
***************
*** 2322,2327 ****
--- 2404,2416 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text template.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      if (verbose)
***************
*** 2383,2388 ****
--- 2472,2484 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text config.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      if (verbose)
          return listTSConfigsVerbose(pattern);

***************
*** 2428,2433 ****
--- 2524,2536 ----
      PGresult   *res;
      int            i;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text config.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,
***************
*** 2504,2509 ****
--- 2607,2619 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text config.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,

Re: Patch to change psql default banner v6

From
Bryce Nesbitt
Date:
Guillaume Lelarge wrote:
> Here is a patch that tries to implement this. Meta-commands should
> work from 7.4 to 8.4-devel releases. It was not hard to do, and I
> don't think it really is a burden to maintain.
>
> One part left to fix is \du and \dg commands. I would be glad to
> continue to work on this but I would prefer to have comments before.
Good work!  I submitted a similar patch also.

For your patch I have one critique: the version sensitive code is
scattered all through describe.c.  Are there opportunities to apply some
tests at a higher level (reducing the number of tests), or gather the
tests into an easily maintainable chunk?

The cleaner it is, the more likely future patchers will continue to
maintain compatibility.

                                  -Bryce Nesbitt


Re: Patch to change psql default banner v6

From
Guillaume Lelarge
Date:
Bryce Nesbitt a écrit :
> Guillaume Lelarge wrote:
>> Here is a patch that tries to implement this. Meta-commands should
>> work from 7.4 to 8.4-devel releases. It was not hard to do, and I
>> don't think it really is a burden to maintain.
>>
>> One part left to fix is \du and \dg commands. I would be glad to
>> continue to work on this but I would prefer to have comments before.
> Good work!  I submitted a similar patch also.

Yes, I've seen that.

> For your patch I have one critique: the version sensitive code is
> scattered all through describe.c.  Are there opportunities to apply some
> tests at a higher level (reducing the number of tests), or gather the
> tests into an easily maintainable chunk?
>

I can't find an easy way to do this. And question is: is it really
interesting to do this? I'm not sure it's worth it. If you want to work
on this and patch my patch, you're welcome :)

> The cleaner it is, the more likely future patchers will continue to
> maintain compatibility.
>

+1

Attached is a new version of the patch. It fixes a few issues when one
adds a pattern to metacommands.

Regards.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
Index: src/bin/psql/describe.c
===================================================================
RCS file: /opt/cvsroot_postgresql/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.173
diff -c -c -r1.173 describe.c
*** src/bin/psql/describe.c    13 May 2008 00:23:17 -0000    1.173
--- src/bin/psql/describe.c    20 May 2008 19:33:27 -0000
***************
*** 59,68 ****
       * There are two kinds of aggregates: ones that work on particular types
       * and ones that work on all (denoted by input type = "any")
       */
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname as \"%s\",\n"
                        "  p.proname AS \"%s\",\n"
!                   "  pg_catalog.format_type(p.prorettype, NULL) AS \"%s\",\n"
                        "  CASE WHEN p.pronargs = 0\n"
                        "    THEN CAST('*' AS pg_catalog.text)\n"
                        "    ELSE\n"
--- 59,75 ----
       * There are two kinds of aggregates: ones that work on particular types
       * and ones that work on all (denoted by input type = "any")
       */
+
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname as \"%s\",\n"
                        "  p.proname AS \"%s\",\n"
!                       "  pg_catalog.format_type(p.prorettype, NULL) AS \"%s\"",
!                       gettext_noop("Schema"),
!                       gettext_noop("Name"),
!                       gettext_noop("Result data type"));
!
!     if (pset.sversion >= 80100)
!         appendPQExpBuffer(&buf, ",\n"
                        "  CASE WHEN p.pronargs = 0\n"
                        "    THEN CAST('*' AS pg_catalog.text)\n"
                        "    ELSE\n"
***************
*** 72,86 ****
                        "      FROM\n"
                        "        pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n"
                        "    ), ', ')\n"
!                       "  END AS \"%s\",\n"
!                  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"\n"
                        "FROM pg_catalog.pg_proc p\n"
         "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n"
                        "WHERE p.proisagg\n",
-                       gettext_noop("Schema"),
-                       gettext_noop("Name"),
-                       gettext_noop("Result data type"),
-                       gettext_noop("Argument data types"),
                        gettext_noop("Description"));

      processSQLNamePattern(pset.db, &buf, pattern, true, false,
--- 79,91 ----
                        "      FROM\n"
                        "        pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n"
                        "    ), ', ')\n"
!                       "  END AS \"%s\"",
!                       gettext_noop("Argument data types"));
!
!     appendPQExpBuffer(&buf, ",\n  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"\n"
                        "FROM pg_catalog.pg_proc p\n"
         "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n"
                        "WHERE p.proisagg\n",
                        gettext_noop("Description"));

      processSQLNamePattern(pset.db, &buf, pattern, true, false,
***************
*** 132,142 ****
                        gettext_noop("Location"));

      if (verbose)
          appendPQExpBuffer(&buf,
!                           ",\n  spcacl AS \"%s\""
!          ",\n  pg_catalog.shobj_description(oid, 'pg_tablespace') AS \"%s\"",
!                           gettext_noop("Access privileges"),
                            gettext_noop("Description"));

      appendPQExpBuffer(&buf,
                        "\nFROM pg_catalog.pg_tablespace\n");
--- 137,151 ----
                        gettext_noop("Location"));

      if (verbose)
+     {
          appendPQExpBuffer(&buf,
!                           ",\n  spcacl AS \"%s\"",
!                           gettext_noop("Access privileges"));
!         if (pset.sversion >= 80200)
!             appendPQExpBuffer(&buf,
!                           ",\n  pg_catalog.shobj_description(oid, 'pg_tablespace') AS \"%s\"",
                            gettext_noop("Description"));
+     }

      appendPQExpBuffer(&buf,
                        "\nFROM pg_catalog.pg_tablespace\n");
***************
*** 179,186 ****
                        "SELECT n.nspname as \"%s\",\n"
                        "  p.proname as \"%s\",\n"
                        "  CASE WHEN p.proretset THEN 'setof ' ELSE '' END ||\n"
!                   "  pg_catalog.format_type(p.prorettype, NULL) as \"%s\",\n"
!                       "  CASE WHEN proallargtypes IS NOT NULL THEN\n"
                        "    pg_catalog.array_to_string(ARRAY(\n"
                        "      SELECT\n"
                        "        CASE\n"
--- 188,201 ----
                        "SELECT n.nspname as \"%s\",\n"
                        "  p.proname as \"%s\",\n"
                        "  CASE WHEN p.proretset THEN 'setof ' ELSE '' END ||\n"
!                   "  pg_catalog.format_type(p.prorettype, NULL) as \"%s\"",
!                       gettext_noop("Schema"),
!                       gettext_noop("Name"),
!                       gettext_noop("Result data type"));
!
!     if (pset.sversion >= 80100)
!         appendPQExpBuffer(&buf,
!                       ",\n  CASE WHEN proallargtypes IS NOT NULL THEN\n"
                        "    pg_catalog.array_to_string(ARRAY(\n"
                        "      SELECT\n"
                        "        CASE\n"
***************
*** 208,216 ****
                        "        pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n"
                        "    ), ', ')\n"
                        "  END AS \"%s\"",
-                       gettext_noop("Schema"),
-                       gettext_noop("Name"),
-                       gettext_noop("Result data type"),
                        gettext_noop("Argument data types"));

      if (verbose)
--- 223,228 ----
***************
*** 220,226 ****
                            "  WHEN p.provolatile = 's' THEN 'stable'\n"
                            "  WHEN p.provolatile = 'v' THEN 'volatile'\n"
                            "END as \"%s\""
!                           ",\n  r.rolname as \"%s\",\n"
                            "  l.lanname as \"%s\",\n"
                            "  p.prosrc as \"%s\",\n"
                    "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
--- 232,238 ----
                            "  WHEN p.provolatile = 's' THEN 'stable'\n"
                            "  WHEN p.provolatile = 'v' THEN 'volatile'\n"
                            "END as \"%s\""
!                           ",\n  pg_catalog.pg_get_userbyid(proowner) as \"%s\",\n"
                            "  l.lanname as \"%s\",\n"
                            "  p.prosrc as \"%s\",\n"
                    "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
***************
*** 238,245 ****
          appendPQExpBuffer(&buf,
                            "\nFROM pg_catalog.pg_proc p"
          "\n     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace"
!              "\n     LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang"
!                 "\n     JOIN pg_catalog.pg_roles r ON r.oid = p.proowner\n");

      /*
       * we skip in/out funcs by excluding functions that take or return cstring
--- 250,256 ----
          appendPQExpBuffer(&buf,
                            "\nFROM pg_catalog.pg_proc p"
          "\n     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace"
!              "\n     LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang\n");

      /*
       * we skip in/out funcs by excluding functions that take or return cstring
***************
*** 254,260 ****
                            "n.nspname", "p.proname", NULL,
                            "pg_catalog.pg_function_is_visible(p.oid)");

!     appendPQExpBuffer(&buf, "ORDER BY 1, 2, 3, 4;");

      res = PSQLexec(buf.data, false);
      termPQExpBuffer(&buf);
--- 265,273 ----
                            "n.nspname", "p.proname", NULL,
                            "pg_catalog.pg_function_is_visible(p.oid)");

!     appendPQExpBuffer(&buf, "ORDER BY 1, 2, 3");
!     if (pset.sversion >= 80100)
!         appendPQExpBuffer(&buf, ", 4;");

      res = PSQLexec(buf.data, false);
      termPQExpBuffer(&buf);
***************
*** 292,298 ****
                        gettext_noop("Schema"),
                        gettext_noop("Name"));
      if (verbose)
!         appendPQExpBuffer(&buf,
                            "  t.typname AS \"%s\",\n"
                            "  CASE WHEN t.typrelid != 0\n"
                            "      THEN CAST('tuple' AS pg_catalog.text)\n"
--- 305,327 ----
                        gettext_noop("Schema"),
                        gettext_noop("Name"));
      if (verbose)
!     {
!         if (pset.sversion < 80300)
!         {
!             appendPQExpBuffer(&buf,
!                           "  t.typname AS \"%s\",\n"
!                           "  CASE WHEN t.typrelid != 0\n"
!                           "      THEN CAST('tuple' AS pg_catalog.text)\n"
!                           "    WHEN t.typlen < 0\n"
!                           "      THEN CAST('var' AS pg_catalog.text)\n"
!                           "    ELSE CAST(t.typlen AS pg_catalog.text)\n"
!                           "  END AS \"%s\",\n",
!                           gettext_noop("Internal name"),
!                           gettext_noop("Size"));
!         }
!         else
!         {
!             appendPQExpBuffer(&buf,
                            "  t.typname AS \"%s\",\n"
                            "  CASE WHEN t.typrelid != 0\n"
                            "      THEN CAST('tuple' AS pg_catalog.text)\n"
***************
*** 312,317 ****
--- 341,348 ----
                            gettext_noop("Internal name"),
                            gettext_noop("Size"),
                            gettext_noop("Elements"));
+         }
+     }

      appendPQExpBuffer(&buf,
                        "  pg_catalog.obj_description(t.oid, 'pg_type') as \"%s\"\n",
***************
*** 419,425 ****

      printfPQExpBuffer(&buf,
                        "SELECT d.datname as \"%s\",\n"
!                       "       r.rolname as \"%s\",\n"
                        "       pg_catalog.pg_encoding_to_char(d.encoding) as \"%s\",\n"
                        "       d.datacl as \"%s\"",
                        gettext_noop("Name"),
--- 450,456 ----

      printfPQExpBuffer(&buf,
                        "SELECT d.datname as \"%s\",\n"
!                       "       pg_catalog.pg_get_userbyid(d.datdba) as \"%s\",\n"
                        "       pg_catalog.pg_encoding_to_char(d.encoding) as \"%s\",\n"
                        "       d.datacl as \"%s\"",
                        gettext_noop("Name"),
***************
*** 428,450 ****
                        gettext_noop("Access Privileges"));
      if (verbose)
      {
!         appendPQExpBuffer(&buf,
                            ",\n       CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
                            "            THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
                            "            ELSE 'No Access'\n"
                            "       END as \"%s\"",
                            gettext_noop("Size"));
!         appendPQExpBuffer(&buf,
                            ",\n       t.spcname as \"%s\"",
                            gettext_noop("Tablespace"));
!         appendPQExpBuffer(&buf,
                            ",\n       pg_catalog.shobj_description(d.oid, 'pg_database') as \"%s\"",
                            gettext_noop("Description"));
      }
      appendPQExpBuffer(&buf,
!                       "\nFROM pg_catalog.pg_database d"
!                       "\n  JOIN pg_catalog.pg_roles r ON d.datdba = r.oid\n");
!     if (verbose)
          appendPQExpBuffer(&buf,
             "  JOIN pg_catalog.pg_tablespace t on d.dattablespace = t.oid\n");
      appendPQExpBuffer(&buf, "ORDER BY 1;");
--- 459,483 ----
                        gettext_noop("Access Privileges"));
      if (verbose)
      {
!         if (pset.sversion >= 80200)
!             appendPQExpBuffer(&buf,
                            ",\n       CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
                            "            THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
                            "            ELSE 'No Access'\n"
                            "       END as \"%s\"",
                            gettext_noop("Size"));
!         if (pset.sversion >= 80100)
!             appendPQExpBuffer(&buf,
                            ",\n       t.spcname as \"%s\"",
                            gettext_noop("Tablespace"));
!         if (pset.sversion >= 80200)
!             appendPQExpBuffer(&buf,
                            ",\n       pg_catalog.shobj_description(d.oid, 'pg_database') as \"%s\"",
                            gettext_noop("Description"));
      }
      appendPQExpBuffer(&buf,
!                       "\nFROM pg_catalog.pg_database d\n");
!     if (verbose && pset.sversion >= 80100)
          appendPQExpBuffer(&buf,
             "  JOIN pg_catalog.pg_tablespace t on d.dattablespace = t.oid\n");
      appendPQExpBuffer(&buf, "ORDER BY 1;");
***************
*** 484,499 ****
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname as \"%s\",\n"
                        "  c.relname as \"%s\",\n"
!                       "  CASE c.relkind WHEN 'r' THEN '%s' WHEN 'v' THEN '%s' WHEN 'S' THEN '%s' END as \"%s\",\n"
!                       "  pg_catalog.array_to_string(c.relacl, E'\\n') as \"%s\"\n"
!                       "FROM pg_catalog.pg_class c\n"
!        "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
!                       "WHERE c.relkind IN ('r', 'v', 'S')\n",
                        gettext_noop("Schema"),
                        gettext_noop("Name"),
                        gettext_noop("table"), gettext_noop("view"), gettext_noop("sequence"),
!                       gettext_noop("Type"),
                        gettext_noop("Access privileges"));

      /*
       * Unless a schema pattern is specified, we suppress system and temp
--- 517,538 ----
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname as \"%s\",\n"
                        "  c.relname as \"%s\",\n"
!                       "  CASE c.relkind WHEN 'r' THEN '%s' WHEN 'v' THEN '%s' WHEN 'S' THEN '%s' END as \"%s\",\n",
                        gettext_noop("Schema"),
                        gettext_noop("Name"),
                        gettext_noop("table"), gettext_noop("view"), gettext_noop("sequence"),
!                       gettext_noop("Type"));
!
!     if (pset.sversion >= 80100)
!         appendPQExpBuffer(&buf, "  pg_catalog.array_to_string(c.relacl, E'\\n') as \"%s\"\n",
!                       gettext_noop("Access privileges"));
!     else
!         appendPQExpBuffer(&buf, "  pg_catalog.array_to_string(c.relacl, '\n') as \"%s\"\n",
                        gettext_noop("Access privileges"));
+
+     appendPQExpBuffer(&buf, "FROM pg_catalog.pg_class c\n"
+        "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
+                       "WHERE c.relkind IN ('r', 'v', 'S')\n");

      /*
       * Unless a schema pattern is specified, we suppress system and temp
***************
*** 984,990 ****
          PGresult   *result;

          printfPQExpBuffer(&buf,
!                           "SELECT i.indisunique, i.indisprimary, i.indisclustered, i.indisvalid, a.amname,
c2.relname,\n"
                      "  pg_catalog.pg_get_expr(i.indpred, i.indrelid, true)\n"
                            "FROM pg_catalog.pg_index i, pg_catalog.pg_class c, pg_catalog.pg_class c2,
pg_catalog.pg_ama\n" 
            "WHERE i.indexrelid = c.oid AND c.oid = '%s' AND c.relam = a.oid\n"
--- 1023,1035 ----
          PGresult   *result;

          printfPQExpBuffer(&buf,
!                           "SELECT i.indisunique, i.indisprimary, i.indisclustered, ");
!         if (pset.sversion < 80000)
!             appendPQExpBuffer(&buf, "true as indisvalid, ");
!         else
!             appendPQExpBuffer(&buf, "i.indisvalid, ");
!
!         appendPQExpBuffer(&buf, "a.amname, c2.relname,\n"
                      "  pg_catalog.pg_get_expr(i.indpred, i.indrelid, true)\n"
                            "FROM pg_catalog.pg_index i, pg_catalog.pg_class c, pg_catalog.pg_class c2,
pg_catalog.pg_ama\n" 
            "WHERE i.indexrelid = c.oid AND c.oid = '%s' AND c.relam = a.oid\n"
***************
*** 1086,1093 ****
          if (tableinfo.hasindex)
          {
              printfPQExpBuffer(&buf,
!                               "SELECT c2.relname, i.indisprimary, i.indisunique, i.indisclustered, i.indisvalid, "
!                               "pg_catalog.pg_get_indexdef(i.indexrelid, 0, true), c2.reltablespace\n"
                                "FROM pg_catalog.pg_class c, pg_catalog.pg_class c2, pg_catalog.pg_index i\n"
                                "WHERE c.oid = '%s' AND c.oid = i.indrelid AND i.indexrelid = c2.oid\n"
                "ORDER BY i.indisprimary DESC, i.indisunique DESC, c2.relname",
--- 1131,1143 ----
          if (tableinfo.hasindex)
          {
              printfPQExpBuffer(&buf,
!                               "SELECT c2.relname, i.indisprimary, i.indisunique, i.indisclustered, ");
!             if (pset.sversion < 80000)
!                 appendPQExpBuffer(&buf, "true as indisvalid, ");
!             else
!                 appendPQExpBuffer(&buf, "i.indisvalid, c2.reltablespace, ");
!
!             appendPQExpBuffer(&buf, "pg_catalog.pg_get_indexdef(i.indexrelid, 0, true)\n"
                                "FROM pg_catalog.pg_class c, pg_catalog.pg_class c2, pg_catalog.pg_index i\n"
                                "WHERE c.oid = '%s' AND c.oid = i.indrelid AND i.indexrelid = c2.oid\n"
                "ORDER BY i.indisprimary DESC, i.indisunique DESC, c2.relname",
***************
*** 1134,1140 ****
                      printTableAddFooter(&cont, buf.data);

                      /* Print tablespace of the index on the same line */
!                     add_tablespace_footer(&cont, 'i',
                                            atooid(PQgetvalue(result, i, 6)),
                                            false);
                  }
--- 1184,1191 ----
                      printTableAddFooter(&cont, buf.data);

                      /* Print tablespace of the index on the same line */
!                     if (pset.sversion >= 80000)
!                         add_tablespace_footer(&cont, 'i',
                                            atooid(PQgetvalue(result, i, 6)),
                                            false);
                  }
***************
*** 1572,1578 ****

      initPQExpBuffer(&buf);

!     appendPQExpBufferStr(&buf,
                           "SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
                           "  r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
                           "  r.rolconnlimit,\n"
--- 1623,1645 ----

      initPQExpBuffer(&buf);

!     if (pset.sversion < 80100)
!     {
!         appendPQExpBufferStr(&buf,
!                       "SELECT u.usename AS rolname,\n"
!                       "  u.usesuper AS rolsuper,\n"
!                       "  FALSE AS rolinherit, FALSE AS rolcreaterole,\n"
!                       "  usecreatedb AS rolcreatedb, TRUE AS rolcanlogin,\n"
!                       "  -1 AS rolconnlimit,\n"
!                       "  ARRAY(SELECT g.groname FROM pg_catalog.pg_group g WHERE u.usesysid = ANY(g.grolist)) as
memberof\n"
!                       "FROM pg_catalog.pg_user u\n");
!
!         processSQLNamePattern(pset.db, &buf, pattern, false, false,
!                           NULL, "u.usename", NULL, NULL);
!     }
!     else
!     {
!         appendPQExpBufferStr(&buf,
                           "SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
                           "  r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
                           "  r.rolconnlimit,\n"
***************
*** 1581,1596 ****
                           "        JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
                           "        WHERE m.member = r.oid) as memberof");

!     if (verbose)
!     {
!         appendPQExpBufferStr(&buf, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
!         ncols++;
!     }

!     appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_roles r\n");

!     processSQLNamePattern(pset.db, &buf, pattern, false, false,
                            NULL, "r.rolname", NULL, NULL);

      appendPQExpBuffer(&buf, "ORDER BY 1;");

--- 1648,1664 ----
                           "        JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
                           "        WHERE m.member = r.oid) as memberof");

!         if (pset.sversion >= 80200 && verbose)
!         {
!             appendPQExpBufferStr(&buf, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
!             ncols++;
!         }

!         appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_roles r\n");

!         processSQLNamePattern(pset.db, &buf, pattern, false, false,
                            NULL, "r.rolname", NULL, NULL);
+     }

      appendPQExpBuffer(&buf, "ORDER BY 1;");

***************
*** 1607,1613 ****
      printTableAddHeader(&cont, gettext_noop("Attributes"), true, align);
      printTableAddHeader(&cont, gettext_noop("Member of"), true, align);

!     if (verbose)
          printTableAddHeader(&cont, gettext_noop("Description"), true, align);

      for (i = 0; i < nrows; i++)
--- 1675,1681 ----
      printTableAddHeader(&cont, gettext_noop("Attributes"), true, align);
      printTableAddHeader(&cont, gettext_noop("Member of"), true, align);

!     if (pset.sversion >= 80200 && verbose)
          printTableAddHeader(&cont, gettext_noop("Description"), true, align);

      for (i = 0; i < nrows; i++)
***************
*** 1650,1656 ****

          printTableAddCell(&cont, PQgetvalue(res, i, 7), false);

!         if (verbose)
              printTableAddCell(&cont, PQgetvalue(res, i, 8), false);
      }
      termPQExpBuffer(&buf);
--- 1718,1724 ----

          printTableAddCell(&cont, PQgetvalue(res, i, 7), false);

!         if (pset.sversion >= 80200 && verbose)
              printTableAddCell(&cont, PQgetvalue(res, i, 8), false);
      }
      termPQExpBuffer(&buf);
***************
*** 1716,1722 ****
                        "SELECT n.nspname as \"%s\",\n"
                        "  c.relname as \"%s\",\n"
                        "  CASE c.relkind WHEN 'r' THEN '%s' WHEN 'v' THEN '%s' WHEN 'i' THEN '%s' WHEN 'S' THEN '%s'
WHEN's' THEN '%s' END as \"%s\",\n" 
!                       "  r.rolname as \"%s\"",
                        gettext_noop("Schema"),
                        gettext_noop("Name"),
                        gettext_noop("table"), gettext_noop("view"), gettext_noop("index"), gettext_noop("sequence"),
gettext_noop("special"),
--- 1784,1790 ----
                        "SELECT n.nspname as \"%s\",\n"
                        "  c.relname as \"%s\",\n"
                        "  CASE c.relkind WHEN 'r' THEN '%s' WHEN 'v' THEN '%s' WHEN 'i' THEN '%s' WHEN 'S' THEN '%s'
WHEN's' THEN '%s' END as \"%s\",\n" 
!                       "  pg_catalog.pg_get_userbyid(c.relowner) as \"%s\"",
                        gettext_noop("Schema"),
                        gettext_noop("Name"),
                        gettext_noop("table"), gettext_noop("view"), gettext_noop("index"), gettext_noop("sequence"),
gettext_noop("special"),
***************
*** 1730,1738 ****

      if (verbose)
      {
!         appendPQExpBuffer(&buf,
                            ",\n  pg_catalog.pg_size_pretty(pg_catalog.pg_relation_size(c.oid)) as \"%s\"",
                              gettext_noop("Size"));
          appendPQExpBuffer(&buf,
                ",\n  pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
                            gettext_noop("Description"));
--- 1798,1809 ----

      if (verbose)
      {
!         if (pset.sversion >= 80100)
!         {
!             appendPQExpBuffer(&buf,
                            ",\n  pg_catalog.pg_size_pretty(pg_catalog.pg_relation_size(c.oid)) as \"%s\"",
                              gettext_noop("Size"));
+         }
          appendPQExpBuffer(&buf,
                ",\n  pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
                            gettext_noop("Description"));
***************
*** 1740,1746 ****

      appendPQExpBuffer(&buf,
                        "\nFROM pg_catalog.pg_class c"
-                     "\n     JOIN pg_catalog.pg_roles r ON r.oid = c.relowner"
       "\n     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace");
      if (showIndexes)
          appendPQExpBuffer(&buf,
--- 1811,1816 ----
***************
*** 1985,1991 ****
      initPQExpBuffer(&buf);
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname AS \"%s\",\n"
!                       "       r.rolname AS \"%s\"",
                        gettext_noop("Name"),
                        gettext_noop("Owner"));

--- 2055,2061 ----
      initPQExpBuffer(&buf);
      printfPQExpBuffer(&buf,
                        "SELECT n.nspname AS \"%s\",\n"
!                       "       pg_catalog.pg_get_userbyid(nspowner) AS \"%s\"",
                        gettext_noop("Name"),
                        gettext_noop("Owner"));

***************
*** 1997,2004 ****
                            gettext_noop("Description"));

      appendPQExpBuffer(&buf,
!               "\nFROM pg_catalog.pg_namespace n JOIN pg_catalog.pg_roles r\n"
!                       "       ON n.nspowner=r.oid\n"
                        "WHERE    (n.nspname !~ '^pg_temp_' OR\n"
             "         n.nspname = (pg_catalog.current_schemas(true))[1])\n");        /* temp schema is first */

--- 2067,2073 ----
                            gettext_noop("Description"));

      appendPQExpBuffer(&buf,
!               "\nFROM pg_catalog.pg_namespace n\n"
                        "WHERE    (n.nspname !~ '^pg_temp_' OR\n"
             "         n.nspname = (pg_catalog.current_schemas(true))[1])\n");        /* temp schema is first */

***************
*** 2035,2040 ****
--- 2104,2116 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text parsers.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      if (verbose)
          return listTSParsersVerbose(pattern);

***************
*** 2083,2088 ****
--- 2159,2171 ----
      PGresult   *res;
      int            i;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text parser.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,
***************
*** 2150,2155 ****
--- 2233,2245 ----
      printQueryOpt myopt = pset.popt;
      static const bool trans_columns[] = {true, false, false};

+     if (pset.sversion < 80000)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text parser.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,
***************
*** 2261,2266 ****
--- 2351,2363 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text dictionary.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,
***************
*** 2322,2327 ****
--- 2419,2431 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text template.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      if (verbose)
***************
*** 2383,2388 ****
--- 2487,2499 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text config.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      if (verbose)
          return listTSConfigsVerbose(pattern);

***************
*** 2428,2433 ****
--- 2539,2551 ----
      PGresult   *res;
      int            i;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text config.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,
***************
*** 2504,2509 ****
--- 2622,2634 ----
      PGresult   *res;
      printQueryOpt myopt = pset.popt;

+     if (pset.sversion < 80300)
+     {
+         fprintf(stderr, _("The server version (%d) does not support full text config.\n"),
+                 pset.sversion);
+         return true;
+     }
+
      initPQExpBuffer(&buf);

      printfPQExpBuffer(&buf,

Re: Patch to change psql default banner v6

From
Bruce Momjian
Date:
David Fetter wrote:
> On Fri, May 16, 2008 at 01:22:55AM -0400, Tom Lane wrote:
> > David Fetter <david@fetter.org> writes:
> > > I believe there's a bug in this patch, namely that the warnings when
> > > there's a server-client mismatch only appear at startup time.
> >
> > Please do not blame this patch for a problem that has been there all
> > along.
> >
> > I don't say that the point doesn't need investigation, but blaming
> > the patch-at-hand for the issue is just misleading.
>
> The patch at hand, as you point out, emphasizes a problem that's been
> there all along, namely that \c doesn't do the same things that
> command line connection does.
>
> I'm volunteering to make them use the same methods :)

David, I have fixed this problem with the attached, applied patch.
Thanks for the observation.  (The patch also removes a duplicate
definition of parse_version().)

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.191
diff -c -c -r1.191 command.c
*** src/bin/psql/command.c    26 Jun 2008 01:35:45 -0000    1.191
--- src/bin/psql/command.c    30 Jun 2008 23:56:53 -0000
***************
*** 29,34 ****
--- 29,37 ----
  #include <sys/types.h>            /* for umask() */
  #include <sys/stat.h>            /* for stat() */
  #endif
+ #ifdef USE_SSL
+ #include <openssl/ssl.h>
+ #endif

  #include "portability/instr_time.h"

***************
*** 57,62 ****
--- 60,74 ----
  static bool do_connect(char *dbname, char *user, char *host, char *port);
  static bool do_shell(const char *command);

+ #ifdef USE_SSL
+ static void printSSLInfo(void);
+ #endif
+
+ #ifdef WIN32
+ static void checkWin32Codepage(void);
+ #endif
+
+

  /*----------
   * HandleSlashCmds:
***************
*** 1185,1190 ****
--- 1197,1203 ----
       * Replace the old connection with the new one, and update
       * connection-dependent variables.
       */
+     connection_warnings();
      PQsetNoticeProcessor(n_conn, NoticeProcessor, NULL);
      pset.db = n_conn;
      SyncVariables();
***************
*** 1212,1217 ****
--- 1225,1324 ----
  }


+ void
+ connection_warnings(void)
+ {
+     if (!pset.quiet && !pset.notty)
+     {
+         int            client_ver = parse_version(PG_VERSION);
+
+         if (pset.sversion != client_ver)
+         {
+             const char *server_version;
+             char        server_ver_str[16];
+
+             /* Try to get full text form, might include "devel" etc */
+             server_version = PQparameterStatus(pset.db, "server_version");
+             if (!server_version)
+             {
+                 snprintf(server_ver_str, sizeof(server_ver_str),
+                          "%d.%d.%d",
+                          pset.sversion / 10000,
+                          (pset.sversion / 100) % 100,
+                          pset.sversion % 100);
+                 server_version = server_ver_str;
+             }
+
+             printf(_("%s (%s, server %s)\n"),
+             pset.progname, PG_VERSION, server_version);
+         }
+         else
+             printf("%s (%s)\n", pset.progname, PG_VERSION);
+
+         if (pset.sversion / 100 != client_ver / 100)
+             printf(_("WARNING: %s version %d.%d, server version %d.%d.\n"
+                  "         Some psql features might not work.\n"),
+                 pset.progname, client_ver / 10000, (client_ver / 100) % 100,
+                 pset.sversion / 10000, (pset.sversion / 100) % 100);
+
+ #ifdef WIN32
+         checkWin32Codepage();
+ #endif
+ #ifdef USE_SSL
+         printSSLInfo();
+ #endif
+     }
+ }
+
+
+ /*
+  * printSSLInfo
+  *
+  * Prints information about the current SSL connection, if SSL is in use
+  */
+ #ifdef USE_SSL
+ static void
+ printSSLInfo(void)
+ {
+     int            sslbits = -1;
+     SSL           *ssl;
+
+     ssl = PQgetssl(pset.db);
+     if (!ssl)
+         return;                    /* no SSL */
+
+     SSL_get_cipher_bits(ssl, &sslbits);
+     printf(_("SSL connection (cipher: %s, bits: %i)\n"),
+            SSL_get_cipher(ssl), sslbits);
+ }
+ #endif
+
+
+ /*
+  * checkWin32Codepage
+  *
+  * Prints a warning when win32 console codepage differs from Windows codepage
+  */
+ #ifdef WIN32
+ static void
+ checkWin32Codepage(void)
+ {
+     unsigned int wincp,
+                 concp;
+
+     wincp = GetACP();
+     concp = GetConsoleCP();
+     if (wincp != concp)
+     {
+         printf(_("WARNING: Console code page (%u) differs from Windows code page (%u)\n"
+                  "         8-bit characters might not work correctly. See psql reference\n"
+                  "         page \"Notes for Windows users\" for details.\n"),
+                concp, wincp);
+     }
+ }
+ #endif
+
+
  /*
   * SyncVariables
   *
Index: src/bin/psql/command.h
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.h,v
retrieving revision 1.30
diff -c -c -r1.30 command.h
*** src/bin/psql/command.h    1 Jan 2008 19:45:55 -0000    1.30
--- src/bin/psql/command.h    30 Jun 2008 23:56:53 -0000
***************
*** 34,39 ****
--- 34,41 ----
          printQueryOpt *popt,
          bool quiet);

+ extern void connection_warnings(void);
+
  extern void SyncVariables(void);

  extern void UnsyncVariables(void);
Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.148
diff -c -c -r1.148 startup.c
*** src/bin/psql/startup.c    16 May 2008 17:17:00 -0000    1.148
--- src/bin/psql/startup.c    30 Jun 2008 23:56:53 -0000
***************
*** 8,16 ****
  #include "postgres_fe.h"

  #include <sys/types.h>
- #ifdef USE_SSL
- #include <openssl/ssl.h>
- #endif

  #ifndef WIN32
  #include <unistd.h>
--- 8,13 ----
***************
*** 78,84 ****
      bool        single_txn;
  };

- static int    parse_version(const char *versionString);
  static void parse_psql_options(int argc, char *argv[],
                     struct adhoc_opts * options);
  static void process_psqlrc(char *argv0);
--- 75,80 ----
***************
*** 86,99 ****
  static void showVersion(void);
  static void EstablishVariableSpace(void);

- #ifdef USE_SSL
- static void printSSLInfo(void);
- #endif
-
- #ifdef WIN32
- static void checkWin32Codepage(void);
- #endif
-
  /*
   *
   * main
--- 82,87 ----
***************
*** 296,344 ****
          if (!options.no_psqlrc)
              process_psqlrc(argv[0]);

          if (!pset.quiet && !pset.notty)
-         {
-             int            client_ver = parse_version(PG_VERSION);
-
-             if (pset.sversion != client_ver)
-             {
-                 const char *server_version;
-                 char        server_ver_str[16];
-
-                 /* Try to get full text form, might include "devel" etc */
-                 server_version = PQparameterStatus(pset.db, "server_version");
-                 if (!server_version)
-                 {
-                     snprintf(server_ver_str, sizeof(server_ver_str),
-                              "%d.%d.%d",
-                              pset.sversion / 10000,
-                              (pset.sversion / 100) % 100,
-                              pset.sversion % 100);
-                     server_version = server_ver_str;
-                 }
-
-                 printf(_("%s (%s, server %s)\n"),
-                 pset.progname, PG_VERSION, server_version);
-             }
-             else
-                 printf("%s (%s)\n", pset.progname, PG_VERSION);
-
-             if (pset.sversion / 100 != client_ver / 100)
-                 printf(_("WARNING: %s version %d.%d, server version %d.%d.\n"
-                      "         Some psql features might not work.\n"),
-                     pset.progname, client_ver / 10000, (client_ver / 100) % 100,
-                     pset.sversion / 10000, (pset.sversion / 100) % 100);
-
- #ifdef WIN32
-             checkWin32Codepage();
- #endif
- #ifdef USE_SSL
-             printSSLInfo();
- #endif
-
              printf(_("Type \"help\" for help.\n\n"));
-         }
-
          if (!pset.notty)
              initializeInput(options.no_readline ? 0 : 1);
          if (options.action_string)        /* -f - was used */
--- 284,292 ----
          if (!options.no_psqlrc)
              process_psqlrc(argv[0]);

+         connection_warnings();
          if (!pset.quiet && !pset.notty)
              printf(_("Type \"help\" for help.\n\n"));
          if (!pset.notty)
              initializeInput(options.no_readline ? 0 : 1);
          if (options.action_string)        /* -f - was used */
***************
*** 358,386 ****


  /*
-  * Convert a version string into a number.
-  */
- static int
- parse_version(const char *versionString)
- {
-     int            cnt;
-     int            vmaj,
-                 vmin,
-                 vrev;
-
-     cnt = sscanf(versionString, "%d.%d.%d", &vmaj, &vmin, &vrev);
-
-     if (cnt < 2)
-         return -1;
-
-     if (cnt == 2)
-         vrev = 0;
-
-     return (100 * vmaj + vmin) * 100 + vrev;
- }
-
-
- /*
   * Parse command line options
   */

--- 306,311 ----
***************
*** 684,737 ****


  /*
-  * printSSLInfo
-  *
-  * Prints information about the current SSL connection, if SSL is in use
-  */
- #ifdef USE_SSL
- static void
- printSSLInfo(void)
- {
-     int            sslbits = -1;
-     SSL           *ssl;
-
-     ssl = PQgetssl(pset.db);
-     if (!ssl)
-         return;                    /* no SSL */
-
-     SSL_get_cipher_bits(ssl, &sslbits);
-     printf(_("SSL connection (cipher: %s, bits: %i)\n"),
-            SSL_get_cipher(ssl), sslbits);
- }
- #endif
-
-
- /*
-  * checkWin32Codepage
-  *
-  * Prints a warning when win32 console codepage differs from Windows codepage
-  */
- #ifdef WIN32
- static void
- checkWin32Codepage(void)
- {
-     unsigned int wincp,
-                 concp;
-
-     wincp = GetACP();
-     concp = GetConsoleCP();
-     if (wincp != concp)
-     {
-         printf(_("WARNING: Console code page (%u) differs from Windows code page (%u)\n"
-                  "         8-bit characters might not work correctly. See psql reference\n"
-                  "         page \"Notes for Windows users\" for details.\n"),
-                concp, wincp);
-     }
- }
- #endif
-
-
- /*
   * Assign hooks for psql variables.
   *
   * This isn't an amazingly good place for them, but neither is anywhere else.
--- 609,614 ----

Re: Patch to change psql default banner v6

From
Tom Lane
Date:
Guillaume Lelarge <guillaume@lelarge.info> writes:
> Attached is a new version of the patch. It fixes a few issues when one
> adds a pattern to metacommands.

Applied with some editorialization.  There were some places you
evidently hadn't tested against all supported server versions ...

            regards, tom lane