Re: Speeding up INSERTs and UPDATEs to partitioned tables

On 5 November 2018 at 20:17, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> On 2018/11/04 19:07, David Rowley wrote:
>> Perhaps a better design would be to instead of having random special
>> partitioned-table-only fields in ResultRelInfo, just have an extra
>> struct there that contains the extra partition information which would
>> include the translation maps and then just return the ResultRelInfo
>> and allow callers to lookup any extra details they require.
>
> IIUC, you're saying that we could introduce a new struct that contains
> auxiliary information needed by partition pruning (maps, slot, etc. for
> tuple conversion) and add a new ResultRelInfo member of that struct type.
> That way, there is no need to return them separately or return an index to
> access them from their arrays. I guess we won't even need the arrays we
> have now. I think that might be a good idea and simplifies things
> significantly.

I've attached a patch which does this. It adds a new struct named
PartitionRoutingInfo into ResultRelInfo and pulls 3 of the 4 arrays
out of PartitionTupleRouting. There are fields for each of what these
arrays used to store inside the PartitionRoutingInfo struct.

While doing this I kept glancing back over at ModifyTableState and at
the mt_per_subplan_tupconv_maps array. I wondered if it would be
better to make the PartitionRoutingInfo a bit more generic, perhaps
call it TupleConversionInfo and have fields like ti_ToGeneric and
ti_FromGeneric, with the idea that "generic" would be the root
partition or the first subplan for inheritance updates. This would
allow us to get rid of a good chunk of code inside nodeModifyTable.c.
However, I ended up not doing this and left PartitionRoutingInfo to be
specifically for Root to Partition conversion.

Also, on the topic about what to name the conversion maps from a few
days ago; After looking at this a bit more I decided that having them
named ParentToChild and ChildToParent is misleading. If the child is
the child of some sub-partitioned table then the parent that the map
is talking about is not the partition's parent, but the root
partitioned table. So really RootToPartition and PartitionToRoot seem
like much more accurate names for the maps.

I made a few other changes along the way; I thought that
ExecFindPartition() would be a good candidate to take on the
responsibility of validating the partition is valid for INSERTs when
it uses a partition out of the subplan_resultrel_hash. I thought it
was better to check this once when we're in the code path of grabbing
the ResultRelInfo out that hash table rather than in a code path that
must check if it's been done already each time we route a tuple into a
partition. It also allowed me to get rid of
ri_PartitionReadyForRouting. I also moved the call to
ExecInitRoutingInfo() into ExecFindPartition() which allowed me to
make that function static, which could result in the generation of
slightly more optimal compiled code.

Please find attached the v14 patch.

Rather nicely git --stat reports a net negative additional code (with
the 48 lines of new tests included)

11 files changed, 632 insertions(+), 660 deletions(-)

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services