On Fri, Mar 31, 2017 at 10:26 PM, Andres Freund <andres@anarazel.de> wrote: > Hi, > > On 2017-04-01 01:22:14 +0000, Robert Haas wrote: >> Avoid GatherMerge crash when there are no workers. > > I think the gather merge code needs a bit more test coverage (sorry to > make this a larger theme today). As shown by > https://coverage.postgresql.org/src/backend/executor/nodeGatherMerge.c.gcov.html > we don't actually merge anything (heap_compare_slots is not exercised).
Sounds reasonable.
I am working on this and will submit the patch soon.
On my local environment I was getting coverage for the heap_compare_slots with existing test. But it seems like due to low number of record fetch only leader get evolved to the fetching tuples in the shared report.
I modified the current test coverage for the Gather Merge to add more rows to be
fetch by the GatherMerge node to make sure that it do coverage for heap_compare_slots. Also added test for the zero worker.
PFA patch as well as LCOV report for the nodeGatherMerge.c.
> I btw also wonder if it's good that we have a nearly identical copy of > heap_compare_slots and a bunch of the calling code in both > nodeMergeAppend.c and nodeGatherMerge.c. On the other hand, it's not > heavily envolving code.
Yeah, I don't know. We could alternatively try to move that to some common location and merge the two implementations. I'm not sure exactly where, though. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
