On Tue, Sep 12, 2017 at 2:35 PM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: > On 2017/09/12 17:53, Ashutosh Bapat wrote: >> On Tue, Sep 12, 2017 at 1:42 PM, Amit Langote wrote: >>> So, we can remove partitioned_rels from (Merge)AppendPath and >>> (Merge)Append nodes and remove ExecLockNonLeafAppendTables(). >> >> Don't we need partitioned_rels from Append paths to be transferred to >> ModifyTable node or we have a different way of calculating >> nonleafResultRelations? > > No, we don't transfer partitioned_rels from Append path to ModifyTable > node. inheritance_planner(), that builds the ModifyTable path for > UPDATE/DELETE on a partitioned table, fetches partitioned_rels from > root->pcinfo_list itself and passes it to create_modifytable_path. No > Append path is involved in that case. PlannedStmt.nonleafResultRelations > is built by concatenating the partitioned_rels lists of all ModifyTable > nodes appearing in the query. It does not depend on Append's or > AppendPath's partitioned_rels.
Ok. Thanks for the explanation. This make me examine inheritance_planner() closely and I think I have spotted a thinko there. In inheritance_planner() parent_rte is set to the RTE of parent to start with and then in the loop 1132 /* 1133 * And now we can get on with generating a plan for each child table. 1134 */ 1135 foreach(lc, root->append_rel_list) 1136 { ... code clipped 1165 /* 1166 * If there are securityQuals attached to the parent, move them to the 1167 * child rel (they've already been transformed properly for that). 1168 */ 1169 parent_rte = rt_fetch(parentRTindex, subroot->parse->rtable); 1170 child_rte = rt_fetch(appinfo->child_relid, subroot->parse->rtable); 1171 child_rte->securityQuals = parent_rte->securityQuals; 1172 parent_rte->securityQuals = NIL; we set parent_rte to the one obtained from subroot->parse, which happens to be the same (at least in contents) as original parent_rte. Later we use this parent_rte to pull partitioned_rels outside that loop 1371 if (parent_rte->relkind == RELKIND_PARTITIONED_TABLE) 1372 { 1373 partitioned_rels = get_partitioned_child_rels(root, parentRTindex); 1374 /* The root partitioned table is included as a child rel */ 1375 Assert(list_length(partitioned_rels) >= 1); 1376 } I think the code here expects the original parent_rte and not the one we set around line 1169. This isn't a bug right now, since both the parent_rte s have same content. But I am not sure if that will remain to be so. Here's patch to fix the thinko. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
inh_planner_prte.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