Re: ExecGather() + nworkers

On Wed, Jan 13, 2016 at 7:19 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Mon, Jan 11, 2016 at 9:16 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
>> On Mon, Jan 11, 2016 at 3:14 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> >
>> > On Sun, Jan 10, 2016 at 9:13 AM, Robert Haas <robertmhaas(at)gmail(dot)com>
>> > wrote:
>> > >> I'm not sure why the test for nworkers following the
>> > >> LaunchParallelWorkers() call doesn't look like this, though:
>> > >>
>> > >> /* Set up tuple queue readers to read the results. */
>> > >> if (pcxt->nworkers_launched > 0)
>> > >> {
>> > >> ...
>> > >> }
>> > >
>> > > Hmm, yeah, I guess it could do that.
>> >
>> > That would make it clearer as an example.
>> >
>> > >> But going to this additional trouble (detecting no workers launched
>> > >> on
>> > >> the basis of !nworkers_launched) suggests that simply testing
>> > >> nworkers_launched would be wrong, which AFAICT it isn't. Can't we
>> > >> just
>> > >> do that, and in so doing also totally remove the "for" loop shown
>> > >> here?
>> > >
>> > > I don't see how the for loop goes away.
>> >
>> > I meant that some code in the "for" loop goes away. Not all of it.
>> > Just the more obscure code. As I said, I'm mostly pointing this out
>> > out of concern for making it clearer as example code.
>> >
>>
>> Right, I can write a patch to do it in a way you are suggesting if you
>> are not planning to do it.
>>
>
> Changed the code such that nworkers_launched gets used wherever
> appropriate instead of nworkers. This includes places other than
> pointed out above.

The changes of the patch are simple optimizations that are trivial.
I didn't find any problem regarding the changes. I think the same
optimization is required in "ExecParallelFinish" function also.

Regards,
Hari Babu
Fujitsu Australia