pg_dump refactor patch to remove global variables - Mailing list pgsql-hackers

From Joachim Wieland
Subject pg_dump refactor patch to remove global variables
Date
Msg-id CACw0+13ZUcXbj9GKJNGZTkym1SXhwRu28nxHoJMoX5Qwmbg_+w@mail.gmail.com
Whole thread Raw
Responses Re: pg_dump refactor patch to remove global variables
Re: pg_dump refactor patch to remove global variables
List 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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: jsonb format is pessimal for toast compression
Next
From: Noah Misch
Date:
Subject: Re: Removing dependency to wsock32.lib when compiling code on WIndows