Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hB0px-0000NC-4x for pgsql-hackers@arkaria.postgresql.org; Mon, 01 Apr 2019 17:35:09 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hB0pu-0005Tw-Pt for pgsql-hackers@arkaria.postgresql.org; Mon, 01 Apr 2019 17:35:06 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hB0pu-0005T4-Fy for pgsql-hackers@lists.postgresql.org; Mon, 01 Apr 2019 17:35:06 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hB0pr-0002Yc-Qv for pgsql-hackers@postgresql.org; Mon, 01 Apr 2019 17:35:06 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id x31HYjj0030708; Mon, 1 Apr 2019 13:34:45 -0400 From: Tom Lane To: Amit Langote cc: David Rowley , Imai Yoshikazu , "jesper.pedersen@redhat.com" , "Imai, Yoshikazu" , Amit Langote , Alvaro Herrera , Robert Haas , Justin Pryzby , Pg Hackers Subject: Re: speeding up planning with partitions In-reply-to: <0b484202-4b89-0797-355f-5c23f786aa6a@lab.ntt.co.jp> References: <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> <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> <23831.1553873385@sss.pgh.pa.us> <0b484! 202-4b89-0797-355f-5c23f786aa6a@lab.ntt.co.jp> Comments: In-reply-to Amit Langote message dated "Mon, 01 Apr 2019 19:02:44 +0900" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <30390.1554139529.0@sss.pgh.pa.us> Date: Mon, 01 Apr 2019 13:34:45 -0400 Message-ID: <30707.1554140085@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <30390.1554139529.1@sss.pgh.pa.us> Content-Transfer-Encoding: quoted-printable Amit Langote 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=3D# create table p (a int) partition by list (a); CREATE TABLE regression=3D# create table p1 partition of p for values in (1); CREATE TABLE regression=3D# set constraint_exclusion to on; SET regression=3D# explain select * from p1 where a =3D 2; QUERY PLAN = ------------------------------------------ Result (cost=3D0.00..0.00 rows=3D0 width=3D0) 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=3D# create table prtab (a int, b int) partition by list (a); CREATE TABLE regression=3D# create table prtab2 partition of prtab for values in (2); CREATE TABLE regression=3D# explain update prtab set b=3Db+1 from p1 where prtab.a=3Dp1= .a and p1.a=3D2; QUERY PLAN = = --------------------------------------------------------------------------= - Update on prtab (cost=3D0.00..82.30 rows=3D143 width=3D20) Update on prtab2 -> Nested Loop (cost=3D0.00..82.30 rows=3D143 width=3D20) -> Seq Scan on p1 (cost=3D0.00..41.88 rows=3D13 width=3D10) Filter: (a =3D 2) -> Materialize (cost=3D0.00..38.30 rows=3D11 width=3D14) -> Seq Scan on prtab2 (cost=3D0.00..38.25 rows=3D11 width= =3D14) Filter: (a =3D 2) (8 rows) No constraint exclusion, while in v10 you get Update on prtab (cost=3D0.00..0.00 rows=3D0 width=3D0) -> Result (cost=3D0.00..0.00 rows=3D0 width=3D0) 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 =3D=3D RELOPT_BASEREL && (root->inhTargetKind =3D=3D INHKIND_NONE || rel->relid !=3D 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 =3D=3D 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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="regression.diffs"; charset="us-ascii" Content-ID: <30390.1554139529.2@sss.pgh.pa.us> Content-Description: regression.diffs Content-Transfer-Encoding: quoted-printable diff -U3 /home/postgres/pgsql/src/test/regress/expected/partition_prune.ou= t /home/postgres/pgsql/src/test/regress/results/partition_prune.out --- /home/postgres/pgsql/src/test/regress/expected/partition_prune.out 201= 9-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 =3D 1) - -> Seq Scan on pp_lp2 - Filter: (a =3D 1) -(7 rows) +(4 rows) = explain (costs off) delete from pp_lp where a =3D 1; QUERY PLAN = -------------------------- Delete on pp_lp Delete on pp_lp1 - Delete on pp_lp2 -> Seq Scan on pp_lp1 Filter: (a =3D 1) - -> Seq Scan on pp_lp2 - Filter: (a =3D 1) -(7 rows) +(4 rows) = set constraint_exclusion =3D 'off'; -- this should not affect the result. explain (costs off) select * from pp_lp where a =3D 1; @@ -3444,24 +3438,18 @@ -------------------------- Update on pp_lp Update on pp_lp1 - Update on pp_lp2 -> Seq Scan on pp_lp1 Filter: (a =3D 1) - -> Seq Scan on pp_lp2 - Filter: (a =3D 1) -(7 rows) +(4 rows) = explain (costs off) delete from pp_lp where a =3D 1; QUERY PLAN = -------------------------- Delete on pp_lp Delete on pp_lp1 - Delete on pp_lp2 -> Seq Scan on pp_lp1 Filter: (a =3D 1) - -> Seq Scan on pp_lp2 - Filter: (a =3D 1) -(7 rows) +(4 rows) = drop table pp_lp; -- Ensure enable_partition_prune does not affect non-partitioned tables. ------- =_aaaaaaaaaa0--