Hi Dilip, Thanks for taking a look.
On 2018/09/03 20:57, Dilip Kumar wrote: > The idea looks interesting while going through the patch I observed > this comment. > > /* > * inheritance_planner > * Generate Paths in the case where the result relation is an > * inheritance set. > * > * We have to handle this case differently from cases where a source relation > * is an inheritance set. Source inheritance is expanded at the bottom of the > * plan tree (see allpaths.c), but target inheritance has to be expanded at > * the top. > > I think with your patch these comments needs to be change? Yes, maybe a good idea to mention that partitioned table result relations are not handled here. Actually, I've been wondering if this patch (0001) shouldn't get rid of inheritance_planner altogether and implement the new approach for *all* inheritance sets, not just partitioned tables, but haven't spent much time on that idea yet. > if (parse->resultRelation && > - rt_fetch(parse->resultRelation, parse->rtable)->inh) > + rt_fetch(parse->resultRelation, parse->rtable)->inh && > + rt_fetch(parse->resultRelation, parse->rtable)->relkind != > + RELKIND_PARTITIONED_TABLE) > inheritance_planner(root); > else > grouping_planner(root, false, tuple_fraction); > > I think we can add some comments to explain if the target rel itself > is partitioned rel then why > we can directly go to the grouping planner. Okay, I will try to add more explanatory comments here and there in the next version I will post later this week. Thanks, Amit