Обсуждение: Re: [COMMITTERS] pgsql: Avoid GatherMerge crash when thereare no workers.
On Sat, Apr 1, 2017 at 7:58 PM, Robert Haas <robertmhaas@gmail.com> wrote:
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.
> 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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Rushabh Lathia
Hi,
--
On Mon, Apr 3, 2017 at 10:56 AM, Rushabh Lathia <rushabh.lathia@gmail.com> wrote:
On Sat, Apr 1, 2017 at 7:58 PM, Robert Haas <robertmhaas@gmail.com> wrote: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/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
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.
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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers --Rushabh Lathia
--
Rushabh Lathia