Re: [HACKERS] UPDATE of partition key - Mailing list pgsql-hackers

On 4 January 2018 at 02:52, David Rowley <david.rowley@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