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 |
Message-ID: | 30707.1554140085@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
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);
CREATE TABLE
regression=# create table p1 partition of p for values in (1);
CREATE TABLE
regression=# set constraint_exclusion to on;
SET
regression=# explain select * from p1 where a = 2;
QUERY PLAN
------------------------------------------
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);
CREATE TABLE
regression=# create table prtab2 partition of prtab for values in (2);
CREATE TABLE
regression=# explain update prtab set b=b+1 from p1 where prtab.a=p1.a and p1.a=2;
QUERY PLAN
---------------------------------------------------------------------------
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 |
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 |