On 4 January 2018 at 02:52, David Rowley <[email protected]> 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
