Re: parallel.c is not marked as test covered

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sun, Jun 19, 2016 at 5:22 PM, Peter Eisentraut
> <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>> Well, the purpose of the test is to check the error passing between worker
>> and leader. If we just silently revert to not doing that, then we can't
>> really be sure that we're testing the right thing.

> I've been thinking about renaming the "single_copy" flag for Gather
> nodes to something like "pipeline" or some other term that might hope
> to convey that the leader won't participate unless no workers can be
> launched at all, and then adding an option to force that to be used
> always. That would allow better testing here,

How so? You still have the fact that functions will run in the leader
when no workers are available, and the gold-plated certainty that that
case will arise routinely in the buildfarm. Perhaps this would move
the probability of running in a worker from 90% to 95%, but I don't
think that'll make much difference from a testing standpoint. Having
said that ...

> although I fear we
> might be getting to a level of tinkering with parallel query that
> starts to look more like feature development.

Personally, I'm +1 for such tinkering if it makes the feature either more
controllable or more understandable. After reading the comments at the
head of nodeGather.c, though, I don't think that single_copy is either
understandable or useful, and merely renaming it won't help. Apparently,
it runs code in the worker, except when it doesn't, and even when it does,
it's absolutely guaranteed to be a performance loss because the leader is
doing nothing. What in the world is the point?

regards, tom lane