Re: [COMMITTERS] pgsql: Avoid GatherMerge crash when there are no workers.

Hi,

On Mon, Apr 3, 2017 at 10:56 AM, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
wrote:

>
>
> On Sat, Apr 1, 2017 at 7:58 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> On Fri, Mar 31, 2017 at 10:26 PM, Andres Freund <andres(at)anarazel(dot)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/nodeGat
>> herMerge.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
>>
>>
>> --
>> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>
>
>
> --
> Rushabh Lathia
>

--
Rushabh Lathia
www.EnterpriseDB.com