Horton

Re: POC: Sharing record typmods between backends

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>, Dilip Kumar <dilipbalaut@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2017-08-23T16:42:47Z
Lists: pgsql-hackers 
On 2017-08-23 09:45:38 -0400, Robert Haas wrote:
> On Wed, Aug 23, 2017 at 1:46 AM, Andres Freund <andres@anarazel.de> wrote:
> > For later commits in the series:
> > - Afaict the whole shared tupledesc stuff, as tqueue.c before, is
> >   entirely untested. This baffles me. See also [1]. I can force the code
> >   to be reached with force_parallel_mode=regress/1, but this absolutely
> >   really totally needs to be reached by the default tests. Robert?
> 
> force_parallel_mode=regress is a good way of testing this because it
> keeps the leader from doing the work, which would likely dodge any
> bugs that happened to exist.  If you want to test something in the
> regular regression tests, using force_parallel_mode=on is probably a
> good way to do it.
> 
> Also note that there are 3 buildfarm members that test with
> force_parallel_mode=regress on a regular basis, so it's not like there
> is no automated coverage of this area.

I don't think that's sufficient. make, and especially check-world,
should have a decent coverage of the code locally. Without having to
know about options like force_parallel_mode=regress. As e.g. evidenced
by the fact that Thomas's latest version crashed if you ran the tests
that way.  If there's a few lines that aren't covered by the plain
tests, and more than a few node + parallelism combinations, I'm not
bothered much. But this is (soon hopefully was) a fairly complicated
piece of infrastructure - that should be exercised.  If necessary that
can just be a BEGIN; SET LOCAL force_parallel_mode=on; query with
blessed descs;COMMIT or whatnot - it's not like we need something hugely
complicated here.

Greetings,

Andres Freund

Commits

  1. Remove TupleDesc remapping logic from tqueue.c.

  2. Add support for coordinating record typmods among parallel workers.

  3. Improve division of labor between execParallel.c and nodeGather[Merge].c.

  4. Add minimal regression test for blessed record type transfer.

  5. Consolidate the function pointer types used by dshash.c.

  6. Fix unlikely shared memory leak after failure in dshash_create().

  7. Refactor typcache.c's record typmod hash table.

  8. Add a hash_combine function for mixing hash values.

  9. Backpatch introduction of TupleDescAttr(tupdesc, i).

  10. Partially flatten struct tupleDesc so that it can be used in DSM.

  11. Change tupledesc->attrs[n] to TupleDescAttr(tupledesc, n).