pg_dump refactor patch to remove global variables

From: Joachim Wieland <joe(at)mcknight(dot)de>
To: pgsql-hackers(at)postgresql(dot)org
Subject: pg_dump refactor patch to remove global variables
Date: 2014-08-15 23:30:54
Message-ID: CACw0+13ZUcXbj9GKJNGZTkym1SXhwRu28nxHoJMoX5Qwmbg_+w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Attached is a patch that doesn't add any new functionality or
features, all it does is get rid of the global variables that
pg_dump.c is full of.

The patch introduces a DumpOptions struct, analogous to the existing
RestoreOptions. This struct then holds the globals but also variables
like dbname, hostname and so on that are currently declared in
pg_dump's main().

Then it passes a pointer to this DumpOptions struct to all functions
that need it.

Motivation for this patch is to a) clean up the globals just because
it's a sane thing to do, b) facilitate future refactoring and c) my
own private objective is to introduce a parallel version of what you
can currently do with "pg_dump | psql" in a later commitfest, where
pg_dump dumps a database in parallel and restores it in parallel
without saving any data on disk. For this, pg_dump needs to handle
more than one database connection which isn't possible with the
current globals and the variables that are declared in main().

One thing I haven't done yet and I'm not sure if it's a good idea to
do, is getting rid of dumputils.c's quote_all_identifiers variable.
This is used by fmtId() which is called in hundreds of places. I don't
need it for my use case but for a proper cleanup of globals one could
make the point that it needs refactoring, too. I didn't want to blow
up the patch size unnecessarily without previous discussion however so
I left it out for now.

There's also more potential refactoring with respect to
DumpOptions/RestoreOptions, they share common fields now which could
be combined and for example a dump in the plain format in pg_dump is
implemented as a restore operation (and goes through the restore
code). This requires access to both dump and restore options. Here as
well, we could do more but for this patch I tried to minimize the
impact for now and kept it close to how it is implemented now.

Attachment Content-Type Size
pg_dump_refactor_globals.diff text/plain 154.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2014-08-16 03:00:01 Re: Removing dependency to wsock32.lib when compiling code on WIndows
Previous Message Tom Lane 2014-08-15 23:19:00 Re: jsonb format is pessimal for toast compression