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? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers