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

On 29 June 2018 at 11:53, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)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