Re: Parallel pg_dump's error reporting doesn't work worth squat

I wrote:
>> BTW, after having spent the afternoon staring at it, I will assert with
>> confidence that pg_dump/parallel.c is an embarrassment to the project.

> I've done a bit of work on a cosmetic cleanup patch, but don't have
> anything to show yet.

Attached is the threatened cosmetic cleanup of parallel.c. As I went
through it, I found quite a few things not to like, but in this patch
I've mostly resisted the temptation to fix them immediately, and have
just tried to document what's there more accurately.

Aside from a major amount of comment-rewriting and a very small amount of
cosmetic code adjustment (mostly moving code for more clarity), this
patch changes these things:

* Rename SetupWorker() to RunWorker() to reflect what it actually does,
and remove its unused "worker" argument.

* Rename lockTableNoWait() to lockTableForWorker() for clarity, and move
the test for BLOBS into it instead of having an Assert that the caller
checked that.

* Don't bother with getThreadLocalPQExpBuffer() at all in non-Windows
builds; it was identical to getLocalPQExpBuffer() anyway, except for
being misleadingly named.

* Remove some completely-redundant or otherwise ill-considered Asserts.

* Fix incorrect "Assert(msgsize <= bufsize)" --- should be < bufsize.

* Fix missing socket close in one error exit from pgpipe(). This isn't
too exciting at present since we'll just curl up and die if it fails,
but might as well get it right.

I have some other, less-cosmetic, things I want to do here, but first
does anyone care to review this?

regards, tom lane