Re: Speeding up INSERTs and UPDATEs to partitioned tables

On 2018/11/01 10:30, David Rowley wrote:
> It's great to know the patch is now so perfect that we've only the
> macro naming left to debate ;-)

I looked over v12 again and noticed a couple minor issues.

+ * table then we store the index into parenting
+ * PartitionTupleRouting 'partition_dispatch_info' array. An

s/PartitionTupleRouting/PartitionTupleRouting's/g

Also, I got a bit concerned about "parenting". Does it mean something
like "enclosing", because the PartitionDispatch is a member of
PartitionTupleRouting? I got concerned because using "parent" like this
may be confusing as this is the partitioning code we're talking about,
where "parent" is generally used to mean "parent" table.

+ * the partitioned table that's the target of the command. If we must
+ * route tuple via some sub-partitioned table, then the PartitionDispatch
+ * for those is only built the first time it's required.

... via some sub-partitioned table"s"

Or perhaps rewrite a bit as:

If we must route the tuple via some sub-partitioned table, then its
PartitionDispatch is built the first time it's required.

The macro naming discussion got me thinking today about the macro itself.
It encapsulates access to the various PartitionTupleRouting arrays
containing the maps, but maybe we've got the interface of tuple routing a
bit (maybe a lot given this thread!) wrong to begin with. Instead of
ExecFindPartition returning indexes into various PartitionTupleRouting
arrays and its callers then using those indexes to fetch various objects
from those arrays, why doesn't it return those objects itself? Although
we made sure that the callers don't need to worry about the meaning of
these indexes changing with this patch, it still seems a bit odd for them
to have to go back to those arrays to get various objects.

How about we change ExecFindPartition's interface so that it returns the
ResultRelInfo, the two maps, and the partition slot? So, the arrays
simply become a cache for ExecFindPartition et al and are no longer
accessed outside execPartition.c. Although that makes the interface of
ExecFindPartition longer, I think it reduces overall complexity.

I've implemented that in the attached patch
1-revise-ExecFindPartition-interface.patch.

Also, since all members of PartitionTupleRouting are only accessed within
execPartition.c save root_tuple_slot, we can move it to execPartition.c to
make its internals private, after doing something about root_tuple_slot.
Looking at the code related to root_tuple_slot, it seems the field really
belongs in ModifyTableState, because it got nothing to do with routing.
Attached 2-make-PartitionTupleRouting-private.patch does that.

These patches 1 and 2 apply on top of v12-0001.. patch.

Thanks,
Amit