On Wed, Mar 8, 2017 at 6:58 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Wed, Mar 8, 2017 at 7:04 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: >> On Wed, Mar 8, 2017 at 8:28 AM, Robert Haas <robertmh...@gmail.com> wrote: >>> On Tue, Mar 7, 2017 at 9:24 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: >>>> I think it can give us benefit in >>>> such cases as well (especially when we have to discard rows based heap >>>> rows). Now, consider it from another viewpoint, what if there are >>>> enough index pages (> min_parallel_index_scan_size) but not sufficient >>>> heap pages. I think in such cases parallel index (only) scans will be >>>> beneficial especially because in the parallel index only scans >>>> heap_pages could be very less or possibly could be zero. >>> >>> That's a separate problem. I think we ought to consider having an >>> index-only scan pass -1 for the number of heap pages, so that the >>> degree of parallelism in that case is limited only by the number of >>> index pages. >> >> Sure, that sounds sensible, but even after that, I am not sure if for >> plain index scans it is a good idea to not choose parallelism if the >> number of heap pages is lesser than min_parallel_table_scan_size even >> though the number of index pages is greater than >> min_parallel_index_scan_size. I think we can try out some tests >> (maybe TPC-H queries where parallel index scan gets picked up) to see >> what is right here. Do you think it will be bad if just commit your >> patch without this change and then consider changing it separately? > > No, I think that would be fine. I think that we need to commit > something like what I proposed because the earlier commit broke > something that used to work. That's got to get fixed. >
Agreed, so I have rebased your patch and passed heap_pages as -1 for index_only scans as discussed. Also, Rafia has tested with attached patch that parallel index and parallel index only scans are picked for TPC-H queries. I have also reviewed and tested your changes with respect to problem reported and found that it works as expected. So, I think we can go ahead with attached patch unless you see some problem with the changes I have made. The only remaining open item about parallel index scans is a decision about storage parameter which is being discussed on parallel index scan thread [1], if you and or others can share feedback, then we can proceed on that aspect as well. [1] - https://www.postgresql.org/message-id/44cd4d75-41f2-8e63-e204-1ecb09127fbf%40archidevsys.co.nz -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
compute-parallel-worker-fix.1.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
