RE: Skip partition tuple routing with constant partition key

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>
Subject: RE: Skip partition tuple routing with constant partition key
Date: 2021-05-24 13:15:35
Message-ID: OS0PR01MB5716F316F03668D4662FA62394269@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Amit-san,

From: Amit Langote <amitlangote09(at)gmail(dot)com>
Sent: Monday, May 24, 2021 4:27 PM
> Hou-san,
>
> On Mon, May 24, 2021 at 10:31 AM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > From: Amit Langote <amitlangote09(at)gmail(dot)com>
> > Sent: Thursday, May 20, 2021 8:23 PM
> > > This one seems bit tough. ExecPartitionCheck() uses the generic
> > > expression evaluation machinery like a black box, which means
> > > execPartition.c can't really tweal/control the time spent evaluating
> > > partition constraints. Given that, we may have to disable the
> > > caching when key->partexprs != NIL, unless we can reasonably do what
> > > you are suggesting.[]
> >
> > I did some research on the CHECK expression that ExecPartitionCheck()
> execute.
>
> Thanks for looking into this and writing the patch. Your idea does sound
> promising.
>
> > 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.

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.

Best regards,
houzj

Attachment Content-Type Size
0001-recursive-search-parent-partkeyexpr.patch application/octet-stream 8.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2021-05-24 13:22:58 Re: rand48 replacement
Previous Message Fabien COELHO 2021-05-24 13:08:57 Re: rand48 replacement