On Thu, Feb 16, 2017 at 1:26 PM, Rahila Syed <rahilasye...@gmail.com> wrote:
> 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. > > True, we do not need this, only removing !index_only_scan should work. Fixed > > > On Thu, Feb 16, 2017 at 1:06 AM, Robert Haas <robertmh...@gmail.com> > wrote: >> >> >> 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_s >> can) >> index_parallelrescan(node->iss_ScanDesc); >> >> And you've got this: >> >> + if (reset_parallel_scan) >> + index_parallelrescan(node->ioss_ScanDesc); >> > Fixed. Please find the attached patch for rebased and cleaner version. -- Regards, Rafia Sabih EnterpriseDB: http://www.enterprisedb.com/
parallel_index_only_v6.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers