On 26 June 2017 at 08:37, Amit Khandekar <amitdkhan...@gmail.com> wrote: > On 22 June 2017 at 01:41, Robert Haas <robertmh...@gmail.com> wrote: >>>> Second, it will amount to a functional bug if you get a >>>> different answer than the planner did. >>> >>> Actually, the per-leaf WCOs are meant to be executed on the >>> destination partitions where the tuple is moved, while the WCOs >>> belonging to the per-subplan resultRelInfo are meant for the >>> resultRelinfo used for the UPDATE plans. So actually it should not >>> matter whether they look same or different, because they are fired at >>> different objects. Now these objects can happen to be the same >>> relations though. >>> >>> But in any case, it's not clear to me how the mapped WCO and the >>> planner's WCO would yield a different answer if they are both the same >>> relation. I am possibly missing something. The planner has already >>> generated the withCheckOptions for each of the resultRelInfo. And then >>> we are using one of those to re-generate the WCO for a leaf partition >>> by only adjusting the attnos. If there is already a WCO generated in >>> the planner for that leaf partition (because that partition was >>> present in mtstate->resultRelInfo), then the re-built WCO should be >>> exactly look same as the earlier one, because they are the same >>> relations, and so the attnos generated in them would be same since the >>> Relation TupleDesc is the same. >> >> If the planner's WCOs and mapped WCOs are always the same, then I >> think we should try to avoid generating both. If they can be >> different, but that's intentional and correct, then there's no >> substantive problem with the patch but the comments need to make it >> clear why we are generating both. >> >>> Actually I meant, "above works for only local updates. For >>> row-movement-updates, we need per-leaf partition WCOs, because when >>> the row is inserted into target partition, that partition may be not >>> be included in the above planner resultRelInfo, so we need WCOs for >>> all partitions". I think this said comment should be sufficient if I >>> add this in the code ? >> >> Let's not get too focused on updating the comment until we are in >> agreement about what the code ought to be doing. I'm not clear >> whether you accept the point that the patch needs to be changed to >> avoid generating the same WCOs and returning lists in both the planner >> and the executor. > > Yes, we can re-use the WCOs generated in the planner, as an > optimization, since those we re-generate for the same relations will > look exactly the same. The WCOs generated by planner (in > inheritance_planner) are generated when (in adjust_appendrel_attrs()) > we change attnos used in the query to refer to the child RTEs and this > adjusts the attnos of the WCOs of the child RTEs. So the WCOs of > subplan resultRelInfo are actually the parent table WCOs, but only the > attnos changed. And in ExecInitModifyTable() we do the same thing for > leaf partitions, although using different function > map_variable_attnos().
In attached patch v12, during UPDATE tuple routing setup, for each leaf partition, we now check if it is present already in one of the UPDATE per-subplan resultrels. If present, we re-use them rather than creating a new one and opening the table again. So the mtstate->mt_partitions is now an array of ResultRelInfo pointers. That pointer points to either the UPDATE per-subplan result rel, or a newly allocated ResultRelInfo. For each of the leaf partitions, we have to search through the per-subplan resultRelInfo oids to check if there is a match. To do this, I have created a temporary hash table which stores oids and the ResultRelInfo pointers of mtstate->resultRelInfo array, and which can be used to search the oid for each of the leaf partitions. This patch version has handled only the above discussion point. I will follow up with the other points separately.
Description: Binary data
-- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers