Thread: psql 8.4 \c repeats version banner

psql 8.4 \c repeats version banner

From
Peter Eisentraut
Date:
In 8.3, running \c from a file prints something like

You are now connected to database "postgres".

In 8.4 it prints

psql (8.4.1)
You are now connected to database "postgres".

Is it intentional/sensible to repeat the startup banner every time the
connection changes, or was this unintentionally introduced while the
startup banner was reshuffled in 8.4?



Re: psql 8.4 \c repeats version banner

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> In 8.3, running \c from a file prints something like
> You are now connected to database "postgres".

> In 8.4 it prints

> psql (8.4.1)
> You are now connected to database "postgres".

> Is it intentional/sensible to repeat the startup banner every time the
> connection changes, or was this unintentionally introduced while the
> startup banner was reshuffled in 8.4?

I think the argument was that if you are connecting to a new server, the
server version number could change, and so it is useful to repeat that
line to have a place to display the possible version mismatch indicator.
Maybe we could suppress it if the third and fourth arguments are
omitted, but I'm not sure it's worth the trouble.

The original design didn't have the third and fourth arguments, hence no
possibility of server change.
        regards, tom lane


Re: psql 8.4 \c repeats version banner

From
Bruce Momjian
Date:
Peter Eisentraut wrote:
> In 8.3, running \c from a file prints something like
>
> You are now connected to database "postgres".
>
> In 8.4 it prints
>
> psql (8.4.1)
> You are now connected to database "postgres".
>
> Is it intentional/sensible to repeat the startup banner every time the
> connection changes, or was this unintentionally introduced while the
> startup banner was reshuffled in 8.4?

I did some reseach on this.  I bet this behavior was added when we
decided to print the backend version warning banner on \c as well as
startup, because it is possible for the backend to be different version
from the backend originally used for psql startup.  The code that prints
the psql banner and the warning banner are in the same function and
share the same output line.

What I did in the attached patch is to add a boolean to
connection_warnings() to indicate whether it was being called on psql
startup or via \c, and to supress the psql banner on \c if the client
and server versions match:

    $ psql test
    psql (8.5devel)
    Type "help" for help.

    test=> \c test
    You are now connected to database "test".
    test=>

Any version mismatch will still print the psql banner for \c, which is
what I think we want.

--
  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.214
diff -c -c -r1.214 command.c
*** src/bin/psql/command.c    5 Feb 2010 03:09:05 -0000    1.214
--- src/bin/psql/command.c    6 Feb 2010 17:22:47 -0000
***************
*** 1331,1337 ****
      PQsetNoticeProcessor(n_conn, NoticeProcessor, NULL);
      pset.db = n_conn;
      SyncVariables();
!     connection_warnings();        /* Must be after SyncVariables */

      /* Tell the user about the new connection */
      if (!pset.quiet)
--- 1331,1337 ----
      PQsetNoticeProcessor(n_conn, NoticeProcessor, NULL);
      pset.db = n_conn;
      SyncVariables();
!     connection_warnings(false);        /* Must be after SyncVariables */

      /* Tell the user about the new connection */
      if (!pset.quiet)
***************
*** 1357,1363 ****


  void
! connection_warnings(void)
  {
      if (!pset.quiet && !pset.notty)
      {
--- 1357,1363 ----


  void
! connection_warnings(bool in_startup)
  {
      if (!pset.quiet && !pset.notty)
      {
***************
*** 1383,1389 ****
              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)
--- 1383,1390 ----
              printf(_("%s (%s, server %s)\n"),
                     pset.progname, PG_VERSION, server_version);
          }
!         /* For version match, only print psql banner on startup. */
!         else if (in_startup)
              printf("%s (%s)\n", pset.progname, PG_VERSION);

          if (pset.sversion / 100 != client_ver / 100)
Index: src/bin/psql/command.h
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.h,v
retrieving revision 1.33
diff -c -c -r1.33 command.h
*** src/bin/psql/command.h    2 Jan 2010 16:57:59 -0000    1.33
--- src/bin/psql/command.h    6 Feb 2010 17:22:47 -0000
***************
*** 34,40 ****
          printQueryOpt *popt,
          bool quiet);

! extern void connection_warnings(void);

  extern void SyncVariables(void);

--- 34,40 ----
          printQueryOpt *popt,
          bool quiet);

! extern void connection_warnings(bool in_startup);

  extern void SyncVariables(void);

Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.160
diff -c -c -r1.160 startup.c
*** src/bin/psql/startup.c    5 Feb 2010 03:09:05 -0000    1.160
--- src/bin/psql/startup.c    6 Feb 2010 17:22:47 -0000
***************
*** 294,300 ****
          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)
--- 294,300 ----
          if (!options.no_psqlrc)
              process_psqlrc(argv[0]);

!         connection_warnings(true);
          if (!pset.quiet && !pset.notty)
              printf(_("Type \"help\" for help.\n\n"));
          if (!pset.notty)

Re: psql 8.4 \c repeats version banner

From
Bruce Momjian
Date:
Applied.

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

Bruce Momjian wrote:
> Peter Eisentraut wrote:
> > In 8.3, running \c from a file prints something like
> > 
> > You are now connected to database "postgres".
> > 
> > In 8.4 it prints
> > 
> > psql (8.4.1)
> > You are now connected to database "postgres".
> > 
> > Is it intentional/sensible to repeat the startup banner every time the
> > connection changes, or was this unintentionally introduced while the
> > startup banner was reshuffled in 8.4?
> 
> I did some reseach on this.  I bet this behavior was added when we
> decided to print the backend version warning banner on \c as well as
> startup, because it is possible for the backend to be different version
> from the backend originally used for psql startup.  The code that prints
> the psql banner and the warning banner are in the same function and
> share the same output line.
> 
> What I did in the attached patch is to add a boolean to
> connection_warnings() to indicate whether it was being called on psql
> startup or via \c, and to supress the psql banner on \c if the client
> and server versions match:
> 
>     $ psql test
>     psql (8.5devel)
>     Type "help" for help.
>     
>     test=> \c test
>     You are now connected to database "test".
>     test=>
> 
> Any version mismatch will still print the psql banner for \c, which is
> what I think we want.
> 
> -- 
>   Bruce Momjian  <bruce@momjian.us>        http://momjian.us
>   EnterpriseDB                             http://enterprisedb.com
> 
>   + If your life is a hard drive, Christ can be your backup. +


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

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +