On Wed, Jan 18, 2017 at 6:55 PM, Rahila Syed <rahilasye...@gmail.com> wrote:
> >+ /* Check if the scan for current scan keys is finished */ > >+ if (so->arrayKeyCount < btscan->btps_arrayKeyCount) > >+ *status = false; > > >I didn't clearly understand, in which scenario the arrayKeyCount is less > >than btps_arrayKeyCount? > Consider following array scan keys > > select * from test2 where j=ANY(ARRAY[1000,2000,3000]); > > By the time the current worker has finished reading heap tuples > corresponding > to array key 1000(arrayKeyCount = 0), some other worker might have > advanced the scan to the > array key 2000(btps_arrayKeyCount =1). In this case when the current > worker fetches next page to scan, > it must advance its scan keys before scanning the next page of parallel > scan. > I hope this helps. > Thanks for the details. One worker incremented arrayKeyCount and btps_arrayKeyCount both. As btps_arrayKeyCount present in shared memory, so the other worker see the update and hits above the condition. > >+BlockNumber > >+_bt_parallel_seize(IndexScanDesc scan, bool *status) > > >The return value of the above function is validated only in _bt_first > >function, but in other cases it is not. > In other cases it is validated in _bt_readnextpage() which is called after > _bt_parallel_seize(). > > >From the function description, > >it is possible to return P_NONE for the workers also with status as > >true. I feel it is better to handle the P_NONE case internally only > >so that callers just check for the status. Am i missing anything? > > In case of the next block being P_NONE and status true, the code > calls _bt_parallel_done() to notify other workers followed by > BTScanPosInvalidate(). Similar check for block = P_NONE also > happens in existing code. See following in _bt_readnextpage(), > > > if (blkno == P_NONE || !so->currPos.moreRight) > { > _bt_parallel_done(scan); > BTScanPosInvalidate(so->currPos); > return false; > } > So to keep it consistent with the existing code, the check > is kept outside _bt_parallel_seize(). > Thanks. Got it. Regards, Hari Babu Fujitsu Australia