From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: David Rowley <david.rowley@2ndquadrant.com>
Cc: Imai Yoshikazu <yoshikazu_i443@live.jp>
Cc: jesper.pedersen@redhat.com <jesper.pedersen@redhat.com>
Cc: Imai, Yoshikazu <imai.yoshikazu@jp.fujitsu.com>
Cc: Amit Langote <amitlangote09@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: speeding up planning with partitions
Date: Mon, 01 Apr 2019 13:34:45 -0400
Message-ID: <30707.1554140085@sss.pgh.pa.us> (raw)
In-Reply-To: <0b484202-4b89-0797-355f-5c23f786aa6a@lab.ntt.co.jp>
References: <CAKJS1f9XAePd2+8F4=wtHKdDdckgKcSoXb-ZbU8Wbt86mhLxKA@mail.gmail.com>
<BYAPR20MB22790FAFA860B03602A9A7B4BC420@BYAPR20MB2279.namprd20.prod.outlook.com>
<d04c7027-2df8-bbd5-9a8f-9806e3ab3270@lab.ntt.co.jp>
<20519.1553288822@sss.pgh.pa.us>
<15573.1553639191@sss.pgh.pa.us>
<47d2fd82-3f70-077b-abc6-863e3fa23503@lab.ntt.co.jp>
<379243a6-33d5-af38-43c9-37120a0893bd@lab.ntt.co.jp>
<a9f76f49-c747-2145-683b-767ff5343d7b@lab.ntt.co.jp>
<CAKJS1f9N6POxkCsq1s_Qzshz-Jmr7Jpq-eJW=ZM=U-SUCPxoMw@mail.gmail.com! >
<ab390249-6aab-195e-bced-e7a427c3462f@lab.ntt.co.jp>
<CAKJS1f-UeJmCLeWgMdcY7Y_Tf2-ZjGFwqHSY2VENYmx2tp2uRQ@mail.gmail.com>
<18712.1553698656@sss.pgh.pa.us>
<5! 9737210-61bd-e583-4393-cc0a5ab52549@lab.ntt.co.jp>
<19663.1553749401@sss.pgh.pa.us>
<2a531b7c-8ced-f868-2e! f2-5cf33b92ef0f@lab.ntt.co.jp>
<23856.1553812684@sss.pgh.pa.us>
<5c83dbca-12b5-1acf-0e85-58299e464a26@lab.ntt.co.jp>
<d807e9fd-2e4c-6910-ade0-1a7e23c706a1@lab.nt! t.co.jp>
<23831.1553873385@sss.pgh.pa.us>
<0b484! 202-4b89-0797-355f-5c23f786aa6a@lab.ntt.co.jp>
Amit Langote <Langote_Amit_f8@lab.ntt.co.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
Attachments:
[text/x-diff] regression.diffs (1.6K, ../30707.1554140085@sss.pgh.pa.us/2-regression.diffs)
download | inline diff:
diff -U3 /home/postgres/pgsql/src/test/regress/expected/partition_prune.out /home/postgres/pgsql/src/test/regress/results/partition_prune.out
--- /home/postgres/pgsql/src/test/regress/expected/partition_prune.out 2019-04-01 12:39:52.613109088 -0400+++ /home/postgres/pgsql/src/test/regress/results/partition_prune.out 2019-04-01 13:18:02.852615395 -0400@@ -3409,24 +3409,18 @@
--------------------------
Update on pp_lp
Update on pp_lp1
- Update on pp_lp2
-> Seq Scan on pp_lp1
Filter: (a = 1)
- -> Seq Scan on pp_lp2- Filter: (a = 1)-(7 rows)+(4 rows)
explain (costs off) delete from pp_lp where a = 1;
QUERY PLAN
--------------------------
Delete on pp_lp
Delete on pp_lp1
- Delete on pp_lp2
-> Seq Scan on pp_lp1
Filter: (a = 1)
- -> Seq Scan on pp_lp2- Filter: (a = 1)-(7 rows)+(4 rows)
set constraint_exclusion = 'off'; -- this should not affect the result.
explain (costs off) select * from pp_lp where a = 1;
@@ -3444,24 +3438,18 @@
--------------------------
Update on pp_lp
Update on pp_lp1
- Update on pp_lp2
-> Seq Scan on pp_lp1
Filter: (a = 1)
- -> Seq Scan on pp_lp2- Filter: (a = 1)-(7 rows)+(4 rows)
explain (costs off) delete from pp_lp where a = 1;
QUERY PLAN
--------------------------
Delete on pp_lp
Delete on pp_lp1
- Delete on pp_lp2
-> Seq Scan on pp_lp1
Filter: (a = 1)
- -> Seq Scan on pp_lp2- Filter: (a = 1)-(7 rows)+(4 rows)
drop table pp_lp;
-- Ensure enable_partition_prune does not affect non-partitioned tables.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, Langote_Amit_f8@lab.ntt.co.jp, david.rowley@2ndquadrant.com, yoshikazu_i443@live.jp, jesper.pedersen@redhat.com, imai.yoshikazu@jp.fujitsu.com, amitlangote09@gmail.com, alvherre@2ndquadrant.com, robertmhaas@gmail.com, pryzby@telsasoft.com
Subject: Re: speeding up planning with partitions
In-Reply-To: <30707.1554140085@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox