On fre, 2011-06-24 at 19:47 -0400, Bruce Momjian wrote:
> > I'm wondering why pg_upgrade needs environment variables at all. It's a
> > one-shot operation. Environment variables are typically used to shared
> > default settings across programs. I don't see how that applies here.
>
> They were there in the original EnterpriseDB code,
Well, the original code wasn't the most stellar example of user
interface engineering, as we have found out over time. ;-)
> and in some cases like PGPORT, we _used_ those environment variables.
Right, and I don't mind using *shared* environment variables for common
settings, when appropriate. But in some cases, as you remember, we had
to work around them or disable them because environment variables
essentially leave traps lying around. So now we are about to create
even more traps.
> Also, the
> command-line can get pretty long so we actually illustrate environment
> variable use in its --help:
>
> For example:
> pg_upgrade -d oldCluster/data -D newCluster/data -b oldCluster/bin -B newCluster/bin
> or
> $ export OLDDATADIR=oldCluster/data
> $ export NEWDATADIR=newCluster/data
> $ export OLDBINDIR=oldCluster/bin
> $ export NEWBINDIR=newCluster/bin
> $ pg_upgrade
I don't see how that makes it better, except that it's somehow prettier
perhaps. There are a lot of programs out there with lots of options. I
have never seen one that creates one-shot environment variables to make
it easier to invoke.
In the above case, you create a bunch of traps. If the user abandons
the attempt to run pg_upgrade but leaves the shell open, comes back at
some other time (or, say, someone else who also logs into the shared
postgres account), and runs just "pg_upgrade" for lack of a better idea
or forgets an option, a destructive operation might start. Yes, they
are stupid and it's their fault and there are other ways to break
things, but pg_upgrade is already tricky enough, we don't need to add
more hidden ways to break it.
(Besides, the above isn't even a portable way to set environment
variables. You need to run the assignment and the export separately.)
> You want the environment variable support removed?
Well, it might be difficult to get consensus on that. I would argue
that we don't need to add new ones for a marginal operation like the
pg_upgrade check mode.
On the other hand, a way to permanently override the new upgrade port
you are working on might be useful. It's not clear from the patch how
to do that, actually.
