Re: Parallel Index Scans

On Fri, Dec 23, 2016 at 5:48 PM, Rahila Syed <rahilasyed90@gmail.com> wrote:
>>> 5. Comment for _bt_parallel_seize() says:
>>> "False indicates that we have reached the end of scan for
>>>  current scankeys and for that we return block number as P_NONE."
>>>
>>>  What is the reason to check (blkno == P_NONE) after checking (status ==
>>> false)
>>>  in _bt_first() (see code below)? If comment is correct
>>>  we'll never reach _bt_parallel_done()
>>>
>>> +               blkno = _bt_parallel_seize(scan, &status);
>>> +               if (status == false)
>>> +               {
>>> +                       BTScanPosInvalidate(so->currPos);
>>> +                       return false;
>>> +               }
>>> +               else if (blkno == P_NONE)
>>> +               {
>>> +                       _bt_parallel_done(scan);
>>> +                       BTScanPosInvalidate(so->currPos);
>>> +                       return false;
>>> +               }
>>>
>>The first time master backend or worker hits last page (calls this
>>API), it will return P_NONE and after that any worker tries to fetch
>>next page, it will return status as false.  I think we can expand a
>>comment to explain it clearly.  Let me know, if you need more
>>clarification, I can explain it in detail.
>
> Probably this was confusing because we have not mentioned
> that P_NONE can be returned even when status = TRUE and
> not just when status is false.
>
> I think, the comment above the function can be modified as follows,
>
> + /*
> + * True indicates that the block number returned is either valid including
> P_NONE
> + * and scan is continued or block number is invalid and scan has just
> + * begun.
>

I think the modification (including P_NONE and scan is continued)
suggested by you can confuse the reader, because if the returned block
number is P_NONE, then we don't continue the scan.  I have used
slightly different words in the patch I have just posted, please check
and see if that looks fine to you.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com