Re: Skip partition tuple routing with constant partition key

Hou-san,

On Mon, May 24, 2021 at 10:15 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
> From: Amit Langote <amitlangote09@gmail.com>
> Sent: Monday, May 24, 2021 4:27 PM
> > On Mon, May 24, 2021 at 10:31 AM houzj.fnst@fujitsu.com
> > <houzj.fnst@fujitsu.com> wrote:
> > > Currently for a normal RANGE partition key it will first generate a
> > > CHECK expression like : [Keyexpression IS NOT NULL AND Keyexpression >
> > lowboud AND Keyexpression < lowboud].
> > > In this case, Keyexpression will be re-executed which will bring some
> > overhead.
> > >
> > > Instead, I think we can try to do the following step:
> > > 1)extract the Keyexpression from the CHECK expression 2)evaluate the
> > > key expression in advance 3)pass the result of key expression to do
> > > the partition CHECK.
> > > In this way ,we only execute the key expression once which looks more
> > efficient.
> >
> > I would have preferred this not to touch anything but ExecPartitionCheck(), at
> > least in the first version.  Especially, seeing that your patch touches
> > partbounds.c makes me a bit nervous, because the logic there is pretty
> > complicated to begin with.
>
> Agreed.
>
> > How about we start with something like the attached?  It's the same idea
> > AFAICS, but implemented with a smaller footprint.  We can consider teaching
> > relcache about this as the next step, if at all.  I haven't measured the
> > performance, but maybe it's not as fast as yours, so will need some fine-tuning.
> > Can you please give it a read?
>
> Thanks for the patch and It looks more compact than mine.
>
> After taking a quick look at the patch, I found a possible issue.
> Currently, the patch does not search the parent's partition key expression recursively.
> For example, If we have multi-level partition:
> Table A is partition of Table B, Table B is partition of Table C.
> It looks like if insert into Table A , then we did not replace the key expression which come from Table C.

Good catch!  Although, I was relieved to realize that it's not *wrong*
per se, as in it does not produce an incorrect result, but only
*slower* than if the patch was careful enough to replace all the
parents' key expressions.

> If we want to get the Table C, we might need to use pg_inherit, but it costs too much to me.
> Instead, maybe we can use the existing logic which already scanned the pg_inherit in function
> generate_partition_qual(). Although this change is out of ExecPartitionCheck(). I think we'd better
> replace all the parents and grandparent...'s key expression.  Attaching a demo patch based on the
> patch you posted earlier. I hope it will help.

Thanks.

Though again, I think we can do this without changing the relcache
interface, such as RelationGetPartitionQual().

PartitionTupleRouting has all the information that's needed here.
Each partitioned table involved in routing a tuple to the leaf
partition has a PartitionDispatch struct assigned to it.  That struct
contains the PartitionKey and we can access partexprs from there.  We
can arrange to assemble them into a single list that is saved to a
given partition's ResultRelInfo, that is, after converting the
expressions to have partition attribute numbers.  I tried that in the
attached updated patch; see the 0002-* patch.

Regarding the first patch to make ExecFindPartition() cache last used
partition, I noticed that it only worked for the bottom-most parent in
a multi-level partition tree, because only leaf partitions were
assigned to dispatch->lastPartitionInfo.  I have fixed the earlier
patch to also save non-leaf partitions and their corresponding
PartitionDispatch structs so that parents of all levels can use this
caching feature.  The patch has to become somewhat complex as a
result, but hopefully not too unreadable.

--
Amit Langote
EDB: http://www.enterprisedb.com