On Wed, Jan 18, 2017 at 6:25 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote: > > > On Mon, Jan 16, 2017 at 11:11 PM, Amit Kapila <amit.kapila16@gmail.com> > wrote: >> > > + * index_beginscan_parallel - join parallel index scan > > The name and the description doesn't sync properly, any better description? >
This can be called by both the worker and leader of parallel index scan. What problem do you see with it. heap_beginscan_parallel has similar description, so not sure changing here alone makes sense.
Ok.
> > +extern BlockNumber _bt_parallel_seize(IndexScanDesc scan, bool *status); > +extern void _bt_parallel_release(IndexScanDesc scan, BlockNumber > scan_page); > > Any better names for the above functions, as these function will provide/set > the next page that needs to be read. >
These functions also set the state of scan. IIRC, these names were being agreed between Robert and Rahila as well (suggested offlist by Robert). I am open to change if you or others have any better suggestions.
I didn't find any better names other than the following,
_bt_get_next_parallel_page
_bt_set_next_parallel_page
> > + /* reset (parallel) index scan */ > + if (node->iss_ScanDesc) > + { > > Why this if check required? There is an assert check in later function > calls. >
This is required because we don't initialize the scan descriptor for parallel-aware nodes during ExecInitIndexScan. It got initialized later at the time of execution when we initialize dsm. Now, it is quite possible that Gather node can occur on inner side of join in which case Rescan can be called before even execution starts. This is the reason why we have similar check in ExecReScanSeqScan which is added during parallel sequential scans (f0661c4e). Does that answer your question?
