On 30 September 2017 at 01:26, Robert Haas <robertmh...@gmail.com> wrote: > On Fri, Sep 29, 2017 at 3:53 PM, Robert Haas <robertmh...@gmail.com> wrote: >> On Fri, Sep 22, 2017 at 1:57 AM, Amit Khandekar <amitdkhan...@gmail.com> >> wrote: >>> The patch for the above change is : >>> 0002-Prevent-a-redundant-ConvertRowtypeExpr-node.patch >> >> Thinking about this a little more, I'm wondering about how this case >> arises. I think that for this patch to avoid multiple conversions, >> we'd have to be calling map_variable_attnos on an expression and then >> calling map_variable_attnos on that expression again.
We are not calling map_variable_attnos() twice. The first time it calls, there is already the ConvertRowtypeExpr node if the expression is a whole row var. This node is already added from adjust_appendrel_attrs(). So the conversion is done by two different functions. For ConvertRowtypeExpr, map_variable_attnos_mutator() recursively calls map_variable_attnos_mutator() for ConvertRowtypeExpr->arg with coerced_var=true. > > I guess I didn't quite finish this thought, sorry. Maybe it's > obvious, but the point I was going for is: why would we do that, vs. > just converting once? The first time ConvertRowtypeExpr node gets added in the expression is when adjust_appendrel_attrs() is called for each of the child tables. Here, for each of the child table, when the parent parse tree is converted into the child parse tree, the whole row var (in RETURNING or WITH CHECK OPTIONS expr) is wrapped with ConvertRowtypeExpr(), so child parse tree (or the child WCO expr) has this ConvertRowtypeExpr node. The second time this node is added is during update-tuple-routing in ExecInitModifyTable(), when map_partition_varattnos() is called for each of the partitions to convert from the first per-subplan RETURNING/WCO expression to the RETURNING/WCO expression belonging to the leaf partition. This second conversion happens for the leaf partitions which are not already present in per-subplan UPDATE result rels. So the first conversion is from parent to child while building per-subplan plans, and the second is from first per-subplan child to another child for building expressions of the leaf partitions. So suppose the root partitioned table RETURNING expression is a whole row var wr(r) where r is its composite type representing the root table type. Then, one of its UPDATE child tables will have its RETURNING expression converted like this : wr(r) ===> CRE(r) -> wr(c1) where CRE(r) represents ConvertRowtypeExpr of result type r, which has its arg pointing to wr(c1) which is a whole row var of composite type c1 for the child table c1. So this node converts from composite type of child table to composite type of root table. Now, when the second conversion occurs for the leaf partition (i.e. during update-tuple-routing), the conversion looks like this : CRE(r) -> wr(c1) ===> CRE(r) -> wr(c2) But W/o the 0002*ConvertRowtypeExpr*.patch the conversion would have looked like this : CRE(r) -> wr(c1) ===> CRE(r) -> CRE(c1) -> wr(c2) In short, we omit the intermediate CRE(c1) node. While writing this down, I observed that after multi-level partition tree expansion was introduced, the child table expressions are not converted directly from the root. Instead, they are converted from their immediate parent. So there is a chain of conversions : to leaf from its parent, to that parent from its parent, and so on from the root. Effectively, during the first conversion, there are that many ConvertRowtypeExpr nodes one above the other already present in the UPDATE result rel expressions. But my patch handles the optimization only for the leaf partition conversions. If already has CRE : CRE(rr) -> wr(r) Parent-to-child conversion ::: CRE(p) -> wr(r) ===> CRE(rr) -> CRE(r) -> wr(c1) W patch : CRE(rr) -> CRE(r) -> wr(c1) ===> CRE(rr) -> CRE(r) -> wr(c2) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers