Re: patch for parallel pg_dump

Excerpts from Joachim Wieland's message of mar abr 03 11:40:31 -0300 2012:
>
> On Tue, Apr 3, 2012 at 9:34 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> > Hmm.  It looks to me like the part-two patch still contains a bunch of
> > code rearrangement.  For example, the current code for
> > pg_backup_archiver.c patch contains this:
> >
> > typedef struct ParallelState
> > {
> >        int                     numWorkers;
> >        ParallelStateEntry *pse;
> > } ParallelState;
> >
> > In the revised patch, that's removed, and parallel.c instead contains this:
> >
> > typedef struct _parallel_state
> > {
> >      int                     numWorkers;
> >      ParallelSlot *parallelSlot;
> > } ParallelState;

> "struct _parallel_state" won't be used anywhere, except for forward
> declarations in headers. I just used it because that seemed to be the
> naming scheme, other structures are called similarly, e.g.:
>
> $ grep "struct _" pg_backup_archiver.c
> typedef struct _restore_args
> typedef struct _parallel_slot
> typedef struct _outputContext
>
> I'm fine with any name, just let me know what you prefer.

I think I can explain this one. In the refactoring patch that Joachim
submitted and I committed, the struct was called _parallel_state, using
the same naming convention with the useless _ suffix and all lowercase
that already plagued the pg_dump code. This name is pointlessly
different from the typedef -- maybe the original pg_dump author thought
the names would collide and chose them different. But this is not true,
looks ugly to me, and furthermore it is inconsistent with what we do in
the rest of the PG code which is to use struct names identical to the
typedef names. So I changed it -- without realizing that the subsequent
patch would move the declarations elsewhere, losing my renaming.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support