On Wed, Sep 13, 2017 at 9:39 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Tue, Sep 12, 2017 at 5:47 PM, Amit Khandekar <amitdkhan...@gmail.com> > wrote: >> On 5 September 2017 at 14:04, Amit Kapila <amit.kapil...@gmail.com> wrote: >> >> I started with a quick review ... a couple of comments below : >> >> - * If this is a baserel, consider gathering any partial paths we may have >> - * created for it. (If we tried to gather inheritance children, we could >> + * If this is a baserel and not the only rel, consider gathering any >> + * partial paths we may have created for it. (If we tried to gather >> >> /* Create GatherPaths for any useful partial paths for rel */ >> - generate_gather_paths(root, rel); >> + if (lev < levels_needed) >> + generate_gather_paths(root, rel, NULL); >> >> I think at the above two places, and may be in other place also, it's >> better to mention the reason why we should generate the gather path >> only if it's not the only rel. >> > > I think the comment you are looking is present where we are calling > generate_gather_paths in grouping_planner. Instead of adding same or > similar comment at multiple places, how about if we just say something > like "See in grouping_planner where we generate gather paths" at all > other places? > >> ---------- >> >> - if (rel->reloptkind == RELOPT_BASEREL) >> - generate_gather_paths(root, rel); >> + if (rel->reloptkind == RELOPT_BASEREL && >> root->simple_rel_array_size > 2) >> + generate_gather_paths(root, rel, NULL); >> >> Above, in case it's a partitioned table, root->simple_rel_array_size >> includes the child rels. So even if it's a simple select without a >> join rel, simple_rel_array_size would be > 2, and so gather path would >> be generated here for the root table, and again in grouping_planner(). >> > > Yeah, that could be a problem. I think we should ensure that there is > no append rel list by checking root->append_rel_list. Can you think > of a better way to handle it? >
The attached patch fixes both the review comments as discussed above. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
parallel_paths_include_tlist_cost_v3.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
