Re: speeding up planning with partitions

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Imai Yoshikazu <yoshikazu_i443(at)live(dot)jp>, "jesper(dot)pedersen(at)redhat(dot)com" <jesper(dot)pedersen(at)redhat(dot)com>, "Imai, Yoshikazu" <imai(dot)yoshikazu(at)jp(dot)fujitsu(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: speeding up planning with partitions
Date: 2019-04-01 17:34:45
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
> On 2019/03/30 0:29, Tom Lane wrote:
>> That seems like probably an independent patch --- do you want to write it?

> Here is that patch.
> It revises get_relation_constraints() such that the partition constraint
> is loaded in only the intended cases.

So I see the problem you're trying to solve here, but I don't like this
patch a bit, because it depends on root->inhTargetKind which IMO is a
broken bit of junk that we need to get rid of. Here is an example of
why, with this patch applied:

regression=# create table p (a int) partition by list (a);
regression=# create table p1 partition of p for values in (1);
regression=# set constraint_exclusion to on;
regression=# explain select * from p1 where a = 2;
Result (cost=0.00..0.00 rows=0 width=0)
One-Time Filter: false
(2 rows)

So far so good, but watch what happens when we include the same case
in an UPDATE on some other partitioned table:

regression=# create table prtab (a int, b int) partition by list (a);
regression=# create table prtab2 partition of prtab for values in (2);
regression=# explain update prtab set b=b+1 from p1 where prtab.a=p1.a and p1.a=2;
Update on prtab (cost=0.00..82.30 rows=143 width=20)
Update on prtab2
-> Nested Loop (cost=0.00..82.30 rows=143 width=20)
-> Seq Scan on p1 (cost=0.00..41.88 rows=13 width=10)
Filter: (a = 2)
-> Materialize (cost=0.00..38.30 rows=11 width=14)
-> Seq Scan on prtab2 (cost=0.00..38.25 rows=11 width=14)
Filter: (a = 2)
(8 rows)

No constraint exclusion, while in v10 you get

Update on prtab (cost=0.00..0.00 rows=0 width=0)
-> Result (cost=0.00..0.00 rows=0 width=0)
One-Time Filter: false

The reason is that this logic supposes that root->inhTargetKind describes
*all* partitioned tables in the query, which is obviously wrong.

Now maybe we could make it work by doing something like

if (rel->reloptkind == RELOPT_BASEREL &&
(root->inhTargetKind == INHKIND_NONE ||
rel->relid != root->parse->resultRelation))

but I find that pretty messy, plus it's violating the concept that we
shouldn't be allowing messiness from inheritance_planner to leak into
other places. What I'd rather do is have this test just read

if (rel->reloptkind == RELOPT_BASEREL)

Making it be that way causes some changes in the partition_prune results,
as attached, which suggest that removing the enable_partition_pruning
check as you did wasn't such a great idea either. However, if I add
that back in, then it breaks the proposed new regression test case.

I'm not at all clear on what we think the interaction between
enable_partition_pruning and constraint_exclusion ought to be,
so I'm not sure what the appropriate resolution is here. Thoughts?

BTW, just about all the other uses of root->inhTargetKind seem equally
broken from here; none of them are accounting for whether the rel in
question is the query target.

regards, tom lane

Attachment Content-Type Size
regression.diffs text/x-diff 1.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-04-01 17:36:58 Re: Extending USE_MODULE_DB to more test suite types
Previous Message Fabien COELHO 2019-04-01 17:26:00 Re: Progress reporting for pg_verify_checksums