Re: About pg_upgrade - Mailing list pgsql-hackers

Peter Eisentraut wrote:
> Bruce Momjian writes:
> 
> > Well, actually, we want the INFODIR and SAVEDATA (was OLDDIR) to be in
> > the same filesystem as PGDATA.  In fact, we use mv so the data files are
> > just moved into different directories and not actually moved.  I added
> > code to put these in the directory _above_ PGDATA.  Does that help?
> 
> Hmm, I think that's actually worse, because the directory above PGDATA
> might in fact be a different file system.  It's probably better to keep it
> within one directory then.  And using "dirname" isn't portable anyway.

Well, actually, it may not be better or worse.  We actually use PGDATA
to point not to the top-level PostgreSQL directory but to the /data part
of that directory, so the old behavior was to put it above /data and the
new code will do exactly that too.  Am I confused?

> > > The way temp files are allocated looks very insecure.  And does this thing
> > > even check under what user it's running?
> >
> > It does now --- It checks for root user and makes sure you can read the
> > PGDATA directory.
> 
> Still not sure about those temp files.  People like to see a possible
> exploit in every temp file.

Well, yes, if you get the pid, you can create symlink files in /tmp and
overwrite things.  How do I handle this properly, probably a directory
in /tmp that I create but I have to set my umask first -- is that a
plan?

> 
> > > 'test -e' is not portable.
> >
> > OK, changed to -f and -h.
> 
> -h isn't portable either.  I think you have to do something like

Oh, great!

> 
> if (test -L "$name") >/dev/null 2>&1 && (test -h "$name") >/dev/null 2>&1

I am confused about this, I don't have -L and the code seems to say that
the second part executes only if the first part is OK --- I don't really
need the test, I could just try the 'mv' and report a failure.

> If should also add here that "if ! command" is not portable, you'd need to
> write "if command; then : ; else action fi"

Wow, I have written portable scripts but never went to these lengths ---
are there actually that many broken OS's?

> > > If you set an exit trap, then 'exit 1' is not portable.  (I'm not kidding,
> > > see pg_regress.)
> >
> > Fixed -- I change 'exit 1' to 'return 1' and changed the calls to:
> >
> >     func || exit "$?"
> 
> That doesn't matter.  You have to write (exit x); exit x so the exit code
> is actually seen outside pg_upgrade.

So 'return 1' doesn't propogate to the caller?

> > > You can't nest " inside ` inside ".
> >
> > Can you show me where?  I don't see it.
> 
> Line 87 looks suspicious.  Especially with the echo and backslash in there
> it is not very portable.

Wow, that command was really stupid --- I just changed it to make the
assignment without the 'echo.'

> 
> > > Moving directories with 'mv' is not necessarily a good idea.
> >
> > Can you be more specific?  Seems pretty portable.
> 
> I was only concerned about moving across file systems, which you're not
> doing.

I sure hope so.  :-)  If not, the script will fail.

> 
> > > Should do a lot more error checking.
> >
> > Suggestions?  Already lots in there.
> 
> It looks better already.

Good.

> > > psql, pg_ctl, etc. should not just be called from the path.  You know the
> > > install directory, so use that.
> >
> > Well, we do use the same script on the old install and the new one so I
> > am not sure if looking at the install or coupling it with 'configure'
> > run is a good idea --- it may cause more problems than it solves.
> 
> It's better than using some random tool that might not even be in the
> path.  You have to run configure anyway, so what's the point in avoiding
> it?

But I don't assume they have run configure on the new source tree when
pg_upgrade is run in phase one  --- very likely they will grab
pg_upgrade, do phase one, then configure/compile the new code and
install it as part of the upgrade steps.

If they don't have a working 'awk' in their path, they have bigger
problems than pg_upgrade not working --- I know there are quirky
problems with awk (I saw them in pgmonitor) but these are vanilla awk
scripts that would work on any one.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026