I reviewed the patch. Overall it looks fine to me. One comment,
>- if (index->amcanparallel && >- !index_only_scan && >+ if ((index->amcanparallel || >+ index_only_scan) && Why do we need to check for index_only_scan in the above condition. IIUC, index->amcanparallel is necessary for parallel index scan to be chosen. We cannot chose parallel index only scan if index_only_scan is happening without worrying about index->amcanparallel value. So OR-ing index->amcanparallel with index_only_scan is probably not correct. Thank you, Rahila Syed On Thu, Feb 16, 2017 at 1:06 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Thu, Jan 19, 2017 at 7:07 AM, Rafia Sabih > <rafia.sa...@enterprisedb.com> wrote: > > Please find the attached file rebased patch of parallel index-only > > scan on the latest Parallel index-scan patch [1]. > > This again needs minor rebasing but basically looks fine. It's a > pretty straightforward extension of the parallel index scan work. > > Please make sure that this is pgindent-clean - i.e. that when you > pgindent the files that it touches, pgindent doesn't change anything > of the same parts of the file that you've changed in the patch. Also, > I believe Amit may have made some adjustments to the logic in > nodeIndexScan.c; if so, it would be good to make sure that the > nodeIndexOnlyScan.c changes match what was done there. In particular, > he's got this: > > if (reset_parallel_scan && node->iss_ScanDesc->parallel_ > scan) > index_parallelrescan(node->iss_ScanDesc); > > And you've got this: > > + if (reset_parallel_scan) > + index_parallelrescan(node->ioss_ScanDesc); > > There might be some other inconsistencies as well that I didn't notice > on a quick look. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
