Re: pg_dump refactor patch to remove global variables

On Thu, Sep 11, 2014 at 5:23 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> I find this rather odd. Can we find a more sensible arrangement? At
> least let's move the extern to dumputils.h, remove it from pg_dump.c.
> Not totally sure about the static in pg_dumpall.c -- note that it does
> call fmtId(), so we might be doing the wrong thing there; maybe we need
> to remove the static from there as well, and let --quote-all-identifiers
> set the one in dumputils.

I've reverted that as proposed, i.e. moved the extern to dumputils.h
and removed the static in pg_dumpall.c.

> I think I like the idea of ConnectionOpts as separate from DumpOptions,
> TBH.

Well, in the end it wasn't all that easy and my initial analysis
wasn't correct. So what I did was that I kind of restored the original
behavior wrt dumpencoding and use_role and pass them as arguments to
setup_connection now as they were before. The only difference now is
that the DumpOptions pointer is also passed in (to setup_connection),
but this makes sense because it needs to check variables in there
which used to be global variables before.

In the new patch I've also taken out other variables from the
DumpOptions struct and made them local in pg_dump.c's main(), partly
proposed by Peter before, those are: numWorkers, prompt_password,
compressLevel, plainText, archiveFormat, archiveMode.

Revised patch attached, thanks for the review Alvaro & Peter.