On 4 January 2018 at 02:52, David Rowley <david.row...@2ndquadrant.com> wrote: > I'll try to look at the tests tomorrow and also do some testing. So > far I've only read the code and the docs.
There are a few more things I noticed on another pass I made today: 20. "carried" -> "carried out the" + would have identified the newly updated row and carried + <command>UPDATE</command>/<command>DELETE</command> on this new row 21. Extra new line + <xref linkend="ddl-partitioning-declarative-limitations">. + </para> 22. In copy.c CopyFrom() you have the following code: /* * We might need to convert from the parent rowtype to the * partition rowtype. */ map = proute->partition_tupconv_maps[leaf_part_index]; if (map) { Relation partrel = resultRelInfo->ri_RelationDesc; tuple = do_convert_tuple(tuple, map); /* * We must use the partition's tuple descriptor from this * point on. Use a dedicated slot from this point on until * we're finished dealing with the partition. */ slot = proute->partition_tuple_slot; Assert(slot != NULL); ExecSetSlotDescriptor(slot, RelationGetDescr(partrel)); ExecStoreTuple(tuple, slot, InvalidBuffer, true); } Should this use ConvertPartitionTupleSlot() instead? 23. Why write; last_resultRelInfo = mtstate->resultRelInfo + mtstate->mt_nplans; when you can write; last_resultRelInfo = mtstate->resultRelInfo[mtstate->mt_nplans];? 24. In ExecCleanupTupleRouting(), do you think that you could just have a special case loop for (mtstate && mtstate->operation == CMD_UPDATE)? /* * If this result rel is one of the UPDATE subplan result rels, let * ExecEndPlan() close it. For INSERT or COPY, this does not apply * because leaf partition result rels are always newly allocated. */ if (is_update && resultRelInfo >= first_resultRelInfo && resultRelInfo < last_resultRelInfo) continue; Something like: if (mtstate && mtstate->operation == CMD_UPDATE) { ResultRelInfo *first_resultRelInfo = mtstate->resultRelInfo; ResultRelInfo *last_resultRelInfo = mtstate->resultRelInfo[mtstate->mt_nplans]; for (i = 0; i < proute->num_partitions; i++) { ResultRelInfo *resultRelInfo = proute->partitions[i]; /* * Leave any resultRelInfos that belong to the UPDATE's subplan * list. These will be closed during executor shutdown. */ if (resultRelInfo >= first_resultRelInfo && resultRelInfo < last_resultRelInfo) continue; ExecCloseIndices(resultRelInfo); heap_close(resultRelInfo->ri_RelationDesc, NoLock); } } else { for (i = 0; i < proute->num_partitions; i++) { ResultRelInfo *resultRelInfo = proute->partitions[i]; ExecCloseIndices(resultRelInfo); heap_close(resultRelInfo->ri_RelationDesc, NoLock); } } -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services