Imai-san, On 2018/11/07 10:00, Imai, Yoshikazu wrote: > About inheritance_make_rel_from_joinlist(), I considered how it processes > joins for sub-partitioned-table. > > sub-partitioned-table image: > part > sub1 > leaf1 > leaf2 > > inheritance_make_rel_from_joinlist() translates join_list and join_info_list > for each leafs(leaf1, leaf2 in above image). To translate those two lists for > leaf1, inheritance_make_rel_from_joinlist() translates lists from part to sub1 > and nextly from sub1 to leaf1. For leaf2, > inheritance_make_rel_from_joinlist() > translates lists from part to sub1 and from sub1 to leaf2. There are same > translation from part to sub1, and I think it is better if we can eliminate > it. > I attached 0002-delta.patch. > > In the patch, inheritance_make_rel_from_joinlist() translates lists not only > for > leafs but for mid-nodes, in a depth-first manner, so it can use lists of > mid-nodes for translating lists from mid-node to leafs, which eliminates same > translation.
I don't think the translation happens twice for the same leaf partitions. Applying adjust_appendrel_attrs_*multilevel* for only leaf nodes, as happens with the current code, is same as first translating using adjust_appendrel_attrs from part to sub1 and then from sub1 to leaf1 and leaf2 during recursion with sub1 as the parent. > I think it might be better if we can apply same logic to > inheritance_planner(), > which once implemented the same logic as below. > > > foreach(lc, root->append_rel_list) > { > AppendRelInfo *appinfo = lfirst_node(AppendRelInfo, lc); > ... > /* > * expand_inherited_rtentry() always processes a parent before any of > * that parent's children, so the parent_root for this relation should > * already be available. > */ > parent_root = parent_roots[appinfo->parent_relid]; > Assert(parent_root != NULL); > parent_parse = parent_root->parse; > ... > subroot->parse = (Query *) > adjust_appendrel_attrs(parent_root, > (Node *) parent_parse, > 1, &appinfo); Actually, inheritance_planner is also using adjust_appendrel_attrs_multilevel. I'm not sure if you're looking at the latest (10/26) patch. Thanks, Amit