Thread: Fix for pg_upgrade status display

Fix for pg_upgrade status display

From
Bruce Momjian
Date:
Pg_upgrade displays file names during copy and database names during
dump/restore.  Andrew Dunstan identified three bugs:

*  long file names were being truncated to 60 _leading_ characters, which
often do not change for long file names

*  file names were truncated to 60 characters in log files

*  carriage returns were being output to log files

The attached patch fixes these --- it prints 60 _trailing_ characters to
the status display, and full path names without carriage returns to log
files.

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

  + It's impossible for everything to be true. +

Attachment

Re: Fix for pg_upgrade status display

From
Robert Haas
Date:
On Wed, Dec 5, 2012 at 10:04 PM, Bruce Momjian <bruce@momjian.us> wrote:
> Pg_upgrade displays file names during copy and database names during
> dump/restore.  Andrew Dunstan identified three bugs:
>
> *  long file names were being truncated to 60 _leading_ characters, which
> often do not change for long file names
>
> *  file names were truncated to 60 characters in log files
>
> *  carriage returns were being output to log files
>
> The attached patch fixes these --- it prints 60 _trailing_ characters to
> the status display, and full path names without carriage returns to log
> files.

This might be a dumb question, but why limit it to 60 characters at
all instead of, say, MAXPGPATH?

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



Re: Fix for pg_upgrade status display

From
Alvaro Herrera
Date:
Robert Haas escribió:
> On Wed, Dec 5, 2012 at 10:04 PM, Bruce Momjian <bruce@momjian.us> wrote:
> > Pg_upgrade displays file names during copy and database names during
> > dump/restore.  Andrew Dunstan identified three bugs:
> >
> > *  long file names were being truncated to 60 _leading_ characters, which
> > often do not change for long file names
> >
> > *  file names were truncated to 60 characters in log files
> >
> > *  carriage returns were being output to log files
> >
> > The attached patch fixes these --- it prints 60 _trailing_ characters to
> > the status display, and full path names without carriage returns to log
> > files.
>
> This might be a dumb question, but why limit it to 60 characters at
> all instead of, say, MAXPGPATH?

I think this should be keyed off the terminal width, actually, no?  The
whole point of this is to overwrite the same line over and over, right?

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services



Re: Fix for pg_upgrade status display

From
Bruce Momjian
Date:
On Thu, Dec  6, 2012 at 12:43:53PM -0500, Robert Haas wrote:
> On Wed, Dec 5, 2012 at 10:04 PM, Bruce Momjian <bruce@momjian.us> wrote:
> > Pg_upgrade displays file names during copy and database names during
> > dump/restore.  Andrew Dunstan identified three bugs:
> >
> > *  long file names were being truncated to 60 _leading_ characters, which
> > often do not change for long file names
> >
> > *  file names were truncated to 60 characters in log files
> >
> > *  carriage returns were being output to log files
> >
> > The attached patch fixes these --- it prints 60 _trailing_ characters to
> > the status display, and full path names without carriage returns to log
> > files.
> 
> This might be a dumb question, but why limit it to 60 characters at
> all instead of, say, MAXPGPATH?

It is limited to 60 only for screen display, so the user knows what is
being processed.  If the text wraps across several lines, the \r trick
to overwrite the string will not work.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +



Re: Fix for pg_upgrade status display

From
Bruce Momjian
Date:
On Thu, Dec  6, 2012 at 02:53:44PM -0300, Alvaro Herrera wrote:
> Robert Haas escribió:
> > On Wed, Dec 5, 2012 at 10:04 PM, Bruce Momjian <bruce@momjian.us> wrote:
> > > Pg_upgrade displays file names during copy and database names during
> > > dump/restore.  Andrew Dunstan identified three bugs:
> > >
> > > *  long file names were being truncated to 60 _leading_ characters, which
> > > often do not change for long file names
> > >
> > > *  file names were truncated to 60 characters in log files
> > >
> > > *  carriage returns were being output to log files
> > >
> > > The attached patch fixes these --- it prints 60 _trailing_ characters to
> > > the status display, and full path names without carriage returns to log
> > > files.
> > 
> > This might be a dumb question, but why limit it to 60 characters at
> > all instead of, say, MAXPGPATH?
> 
> I think this should be keyed off the terminal width, actually, no?  The
> whole point of this is to overwrite the same line over and over, right?

That seems like overkill for a status message.  It is just there so
users know pg_upgrade isn't stuck, which was the complaint before the
message was used.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +



Re: Fix for pg_upgrade status display

From
Bruce Momjian
Date:
On Wed, Dec  5, 2012 at 10:04:53PM -0500, Bruce Momjian wrote:
> Pg_upgrade displays file names during copy and database names during
> dump/restore.  Andrew Dunstan identified three bugs:
> 
> *  long file names were being truncated to 60 _leading_ characters, which
> often do not change for long file names
> 
> *  file names were truncated to 60 characters in log files
> 
> *  carriage returns were being output to log files
> 
> The attached patch fixes these --- it prints 60 _trailing_ characters to
> the status display, and full path names without carriage returns to log
> files.

Patch applied.   It also suppresses status output to the log file unless
verbose mode is used.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +