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 1h7DHS-00023Q-3h for pgsql-hackers@arkaria.postgresql.org; Fri, 22 Mar 2019 06:03:50 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1h7DHQ-0000Uj-Gq for pgsql-hackers@arkaria.postgresql.org; Fri, 22 Mar 2019 06:03:48 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1h7DHP-0000Q7-WD for pgsql-hackers@lists.postgresql.org; Fri, 22 Mar 2019 06:03:48 +0000 Received: from tama50.ecl.ntt.co.jp ([129.60.39.147]) by makus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1h7DHI-0003Vd-Es for pgsql-hackers@lists.postgresql.org; Fri, 22 Mar 2019 06:03:46 +0000 Received: from vc1.ecl.ntt.co.jp (vc1.ecl.ntt.co.jp [129.60.86.153]) by tama50.ecl.ntt.co.jp (8.13.8/8.13.8) with ESMTP id x2M63VLT017211; Fri, 22 Mar 2019 15:03:31 +0900 Received: from vc1.ecl.ntt.co.jp (localhost [127.0.0.1]) by vc1.ecl.ntt.co.jp (Postfix) with ESMTP id D5747EA7D46; Fri, 22 Mar 2019 15:03:31 +0900 (JST) Received: from jcms-pop11.ecl.ntt.co.jp (jcms-pop11.ecl.ntt.co.jp [129.60.87.132]) by vc1.ecl.ntt.co.jp (Postfix) with ESMTP id CAA6AEA7D41; Fri, 22 Mar 2019 15:03:31 +0900 (JST) Received: from hosoyahp (unknown [129.60.241.74]) by jcms-pop11.ecl.ntt.co.jp (Postfix) with ESMTPSA id C4C2F70C0769; Fri, 22 Mar 2019 15:03:31 +0900 (JST) From: "Yuzuko Hosoya" References: <00e601d4ca86$932b8bc0$b982a340$@lab.ntt.co.jp> <010e01d4ce68$c0427b10$40c77130$@lab.ntt.co.jp> <0F97FA9ABBDBE54F91744A9B37151A5129A78A@g01jpexmbkw24> <001901d4daf5$1ef4f640$5cdee2c0$@lab.ntt.co.jp> <9bed6b79-f264-6976-b880-e2a5d23e9d85@lab.ntt.co.jp> <003501d4de21$8816f540$9844dfc0$@lab.ntt.co.jp> <9bb31dfe-b0d0-53f3-3ea6-e64b811424cf@lab.ntt.co.jp> In-Reply-To: Subject: RE: Problem with default partition pruning Date: Fri, 22 Mar 2019 15:02:51 +0900 Message-ID: <005101d4e074$e24d1e40$a6e75ac0$@lab.ntt.co.jp> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0052_01D4E0C0.52351460" X-Mailer: Microsoft Outlook 15.0 Content-language: ja Thread-index: AQLwSThp7L0tC61luGAd4dJt3HxY3QJvrgn9AgtGZCUBpM85VAEOZ04CAeo+SK4CDbFrsgGXrNc6AV8LXc4CAoWeVAJ/vK7zo0tVuXA= X-CC-Mail-RelayStamp: 1 To: "'Thibaut'" , "'Amit Langote'" , "'Imai, Yoshikazu'" Cc: "'PostgreSQL Hackers'" X-TM-AS-MML: disable List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk This is a multipart message in MIME format. ------=_NextPart_000_0052_01D4E0C0.52351460 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, Thanks a lot for additional tests and the new patch. > Le 20/03/2019 =C3=A0 10:06, Amit Langote a =C3=A9crit : > > Hi Thibaut, > > > > On 2019/03/19 23:58, Thibaut Madelaine wrote: > >> I kept on testing with sub-partitioning. > > Thanks. > > > >> I found a case, using 2 default partitions, where a default = partition > >> is not pruned: > >> > >> -------------- > >> > >> create table test2(id int, val text) partition by range (id); = create > >> table test2_20_plus_def partition of test2 default; create table > >> test2_0_20 partition of test2 for values from (0) to (20) > >> partition by range (id); > >> create table test2_0_10 partition of test2_0_20 for values from (0) > >> to (10); create table test2_10_20_def partition of test2_0_20 > >> default; > >> > >> # explain (costs off) select * from test2 where id=3D5 or id=3D25; > >> QUERY PLAN > >> ----------------------------------------- > >> Append > >> -> Seq Scan on test2_0_10 > >> Filter: ((id =3D 5) OR (id =3D 25)) > >> -> Seq Scan on test2_10_20_def > >> Filter: ((id =3D 5) OR (id =3D 25)) > >> -> Seq Scan on test2_20_plus_def > >> Filter: ((id =3D 5) OR (id =3D 25)) > >> (7 rows) > >> > >> -------------- > >> > >> I have the same output using Amit's v1-delta.patch or Hosoya's > >> v2_default_partition_pruning.patch. > > I think I've figured what may be wrong. > > > > Partition pruning step generation code should ignore any arguments = of > > an OR clause that won't be true for a sub-partitioned partition, = given > > its partition constraint. > > > > In this case, id =3D 25 contradicts test2_0_20's partition = constraint > > (which is, a IS NOT NULL AND a >=3D 0 AND a < 20), so the OR clause > > should really be simplified to id =3D 5, ignoring the id =3D 25 = argument. > > Note that we remove id =3D 25 only for the considerations of pruning = and > > not from the actual clause that's passed to the final plan, although > > it wouldn't be a bad idea to try to do that. > > > > Attached revised delta patch, which includes the fix described = above. > > > > Thanks, > > Amit > Amit, I tested many cases with nested range sub-partitions... and I = did not find any problem with your > last patch :-) >=20 > I tried mixing with hash partitions with no problems. >=20 > From the patch, there seems to be less checks than before. I cannot = think of a case that can have > performance impacts. >=20 > Hosoya-san, if you agree with Amit's proposal, do you think you can = send a patch unifying your > default_partition_pruning.patch and Amit's second v1-delta.patch? > I understood Amit's proposal. But I think the issue Thibaut reported = would=20 occur regardless of whether clauses have OR clauses or not as follows. I tested a query which should output "One-Time Filter: false". # explain select * from test2_0_20 where id =3D 25; QUERY PLAN =20 ----------------------------------------------------------------------- Append (cost=3D0.00..25.91 rows=3D6 width=3D36) -> Seq Scan on test2_10_20_def (cost=3D0.00..25.88 rows=3D6 = width=3D36) Filter: (id =3D 25) As Amit described in the previous email, id =3D 25 contradicts = test2_0_20's partition constraint, so I think this clause should be ignored and we = can also handle this case in the similar way as Amit proposal. I attached v1-delta-2.patch which fix the above issue. =20 What do you think about it? Best regards, Yuzuko Hosoya ------=_NextPart_000_0052_01D4E0C0.52351460 Content-Type: application/octet-stream; name="v1-delta-2.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="v1-delta-2.patch" diff --git a/src/backend/partitioning/partprune.c = b/src/backend/partitioning/partprune.c=0A= index 5c1632f..d8e0043 100644=0A= --- a/src/backend/partitioning/partprune.c=0A= +++ b/src/backend/partitioning/partprune.c=0A= @@ -822,32 +822,13 @@ = gen_partprune_steps_internal(GeneratePruningStepsContext *context,=0A= * an arg. To indicate that to the pruning code, we=0A= * must construct a dummy PartitionPruneStepCombine=0A= * whose source_stepids is set to an empty List.=0A= - * However, if we can prove using constraint exclusion=0A= - * that the clause refutes the table's partition=0A= - * constraint (if it's sub-partitioned), we need not=0A= - * bother with that. That is, we effectively ignore=0A= - * this OR arm.=0A= */=0A= - List *partconstr =3D rel->partition_qual;=0A= PartitionPruneStep *orstep;=0A= =0A= /* Just ignore this argument. */=0A= if (arg_contradictory)=0A= continue;=0A= =0A= - if (partconstr)=0A= - {=0A= - partconstr =3D (List *)=0A= - expression_planner((Expr *) partconstr);=0A= - if (rel->relid !=3D 1)=0A= - ChangeVarNodes((Node *) partconstr, 1,=0A= - rel->relid, 0);=0A= - if (predicate_refuted_by(partconstr,=0A= - list_make1(arg),=0A= - false))=0A= - continue;=0A= - }=0A= -=0A= orstep =3D gen_prune_step_combine(context, NIL,=0A= PARTPRUNE_COMBINE_UNION);=0A= arg_stepids =3D lappend_int(arg_stepids, orstep->step_id);=0A= @@ -923,6 +904,7 @@ = gen_partprune_steps_internal(GeneratePruningStepsContext *context,=0A= bool clause_is_not_null =3D false;=0A= PartClauseInfo *pc =3D NULL;=0A= List *clause_steps =3D NIL;=0A= + List *partconstr =3D rel->partition_qual;=0A= =0A= switch (match_clause_to_partition_key(rel, context,=0A= clause, partkey, i,=0A= @@ -933,6 +915,29 @@ = gen_partprune_steps_internal(GeneratePruningStepsContext *context,=0A= Assert(pc !=3D NULL);=0A= =0A= /*=0A= + * If this clause can be proved false for this partition,=0A= + * given its partition constraint, we can ignore it, =0A= + * that is not try to pass it to the pruning code. =0A= + * We should do that especially to avoid pruning code =0A= + * wrongly failing to prune the default partition.=0A= + */=0A= + if (partconstr)=0A= + {=0A= + partconstr =3D (List *)=0A= + expression_planner((Expr *) partconstr);=0A= + if (rel->relid !=3D 1)=0A= + ChangeVarNodes((Node *) partconstr, 1,=0A= + rel->relid, 0);=0A= + if (predicate_refuted_by(partconstr,=0A= + list_make1(clause),=0A= + false))=0A= + {=0A= + *contradictory =3D true;=0A= + return NIL;=0A= + }=0A= + }=0A= + =0A= + /*=0A= * Since we only allow strict operators, check for any=0A= * contradicting IS NULL.=0A= */=0A= @@ -2759,15 +2764,19 @@ get_matching_range_bounds(PartitionPruneContext = *context,=0A= * instead as the offset of the upper bound of the greatest=0A= * partition that may contain lookup value. If the lookup=0A= * value had exactly matched the bound, but it isn't=0A= - * inclusive, no need add the adjacent partition. If 'off' is=0A= - * -1 indicating that all bounds are greater, then we simply=0A= - * end up adding the first bound's offset, that is, 0.=0A= + * inclusive, no need add the adjacent partition.=0A= */=0A= - else if (off < 0 || !is_equal || inclusive)=0A= + else if (!is_equal || inclusive)=0A= maxoff =3D off + 1;=0A= else=0A= maxoff =3D off;=0A= }=0A= + else=0A= + /*=0A= + * 'off' is -1 indicating that all bounds are greater, so just=0A= + * set the first bound's offset as maxoff.=0A= + */=0A= + maxoff =3D off + 1;=0A= break;=0A= =0A= default:=0A= ------=_NextPart_000_0052_01D4E0C0.52351460--