Re: partitioning - changing a slot's descriptor is expensive

On 29 June 2018 at 11:53, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> What I'm thinking of doing is something that's inspired by one of the
> things that David Rowley proposes in his patch for PG 12 to remove
> inefficiencies in the tuple routing code [1].
>
> Instead of a single TupleTableSlot attached at partition_tuple_slot, we
> allocate an array of TupleTableSlot pointers of same length as the number
> of partitions, as you mentioned upthread.  We then call
> MakeTupleTableSlot() only if a partition needs it and pass it the
> partition's TupleDesc.  Allocated slots are remembered in a list.
> ExecDropSingleTupleTableSlot is then called on those allocated slots when
> the plan ends.  Note that the array of slots is not allocated if none of
> the partitions affected by a given query (or copy) needed to convert tuples.

Thanks for the patch. I did some review of the patch (needs a rebase).
Below are my comments.

@@ -1936,12 +1936,11 @@ ExecPartitionCheckEmitError(ResultRelInfo
*resultRelInfo,
+   /* Input slot might be of a partition, which has a fixed tupdesc. */
+   slot = MakeTupleTableSlot(tupdesc);
    if (map != NULL)
-   {
      tuple = do_convert_tuple(tuple, map);
-     ExecSetSlotDescriptor(slot, tupdesc);
-     ExecStoreTuple(tuple, slot, InvalidBuffer, false);
-   }
+   ExecStoreTuple(tuple, slot, InvalidBuffer, false);

Both MakeTupleTableSlot() and ExecStoreTuple() can be inside the (map
!= NULL) if condition.
This also applies for similar changes in ExecConstraints() and
ExecWithCheckOptions().

+ * Initialize an empty slot that will be used to manipulate tuples of any
+ * this partition's rowtype.
of any this => of this

+ * Initialized the array where these slots are stored, if not already
Initialized => Initialize

+ proute->partition_tuple_slots_alloced =
+ lappend(proute->partition_tuple_slots_alloced,
+ proute->partition_tuple_slots[partidx]);

Instead of doing the above, I think we can collect those slots in
estate->es_tupleTable using ExecInitExtraTupleSlot() so that they
don't have to be explicitly dropped in ExecCleanupTupleRouting(). And
also then we won't require the new field
proute->partition_tuple_slots_alloced.


-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company