From: Yuzuko Hosoya <hosoya.yuzuko@lab.ntt.co.jp>
To: 'Thibaut' <thibaut.madelaine@dalibo.com>
To: 'Amit Langote' <Langote_Amit_f8@lab.ntt.co.jp>
To: 'Imai, Yoshikazu' <imai.yoshikazu@jp.fujitsu.com>
Cc: 'PostgreSQL Hackers' <pgsql-hackers@lists.postgresql.org>
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> (raw)
In-Reply-To: <c6b9557e-c029-e72c-c697-3f216e559806@dalibo.com>
References: <00e601d4ca86$932b8bc0$b982a340$@lab.ntt.co.jp>
<afef4fb8-c448-d54a-9628-7c26277d4895@lab.ntt.co.jp>
<010e01d4ce68$c0427b10$40c77130$@lab.ntt.co.jp>
<0F97FA9ABBDBE54F91744A9B37151A5129A78A@g01jpexmbkw24>
<a4968068-6401-7a9c-8bd4-6a3bc9164a86@dalibo.com>
<001901d4daf5$1ef4f640$5cdee2c0$@lab.ntt.co.jp>
<9bed6b79-f264-6976-b880-e2a5d23e9d85@lab.ntt.co.jp>
<003501d4de21$8816f540$9844dfc0$@lab.ntt.co.jp>
<bd03f475-30d4-c4d0-3d7f-d2fbde755971@dalibo.com>
<9bb31dfe-b0d0-53f3-3ea6-e64b811424cf@lab.ntt.co.jp>
<c6b9557e-c029-e72c-c697-3f216e559806@dalibo.com>
Hi,
Thanks a lot for additional tests and the new patch.
> Le 20/03/2019 à 10:06, Amit Langote a écrit :
> > 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=5 or id=25;
> >> QUERY PLAN
> >> -----------------------------------------
> >> Append
> >> -> Seq Scan on test2_0_10
> >> Filter: ((id = 5) OR (id = 25))
> >> -> Seq Scan on test2_10_20_def
> >> Filter: ((id = 5) OR (id = 25))
> >> -> Seq Scan on test2_20_plus_def
> >> Filter: ((id = 5) OR (id = 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 = 25 contradicts test2_0_20's partition constraint
> > (which is, a IS NOT NULL AND a >= 0 AND a < 20), so the OR clause
> > should really be simplified to id = 5, ignoring the id = 25 argument.
> > Note that we remove id = 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 :-)
>
> I tried mixing with hash partitions with no problems.
>
> From the patch, there seems to be less checks than before. I cannot think of a case that can have
> performance impacts.
>
> 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
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 = 25;
QUERY PLAN
-----------------------------------------------------------------------
Append (cost=0.00..25.91 rows=6 width=36)
-> Seq Scan on test2_10_20_def (cost=0.00..25.88 rows=6 width=36)
Filter: (id = 25)
As Amit described in the previous email, id = 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.
What do you think about it?
Best regards,
Yuzuko Hosoya
Attachments:
[application/octet-stream] v1-delta-2.patch (3.5K, ../005101d4e074$e24d1e40$a6e75ac0$@lab.ntt.co.jp/2-v1-delta-2.patch)
download | inline diff:diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.cindex 5c1632f..d8e0043 100644--- a/src/backend/partitioning/partprune.c+++ b/src/backend/partitioning/partprune.c@@ -822,32 +822,13 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
* an arg. To indicate that to the pruning code, we
* must construct a dummy PartitionPruneStepCombine
* whose source_stepids is set to an empty List.
- * However, if we can prove using constraint exclusion- * that the clause refutes the table's partition- * constraint (if it's sub-partitioned), we need not- * bother with that. That is, we effectively ignore- * this OR arm.
*/
- List *partconstr = rel->partition_qual;
PartitionPruneStep *orstep;
/* Just ignore this argument. */
if (arg_contradictory)
continue;
- if (partconstr)- {- partconstr = (List *)- expression_planner((Expr *) partconstr);- if (rel->relid != 1)- ChangeVarNodes((Node *) partconstr, 1,- rel->relid, 0);- if (predicate_refuted_by(partconstr,- list_make1(arg),- false))- continue;- }-
orstep = gen_prune_step_combine(context, NIL,
PARTPRUNE_COMBINE_UNION);
arg_stepids = lappend_int(arg_stepids, orstep->step_id);
@@ -923,6 +904,7 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
bool clause_is_not_null = false;
PartClauseInfo *pc = NULL;
List *clause_steps = NIL;
+ List *partconstr = rel->partition_qual;
switch (match_clause_to_partition_key(rel, context,
clause, partkey, i,
@@ -933,6 +915,29 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
Assert(pc != NULL);
/*
+ * If this clause can be proved false for this partition,+ * given its partition constraint, we can ignore it, + * that is not try to pass it to the pruning code. + * We should do that especially to avoid pruning code + * wrongly failing to prune the default partition.+ */+ if (partconstr)+ {+ partconstr = (List *)+ expression_planner((Expr *) partconstr);+ if (rel->relid != 1)+ ChangeVarNodes((Node *) partconstr, 1,+ rel->relid, 0);+ if (predicate_refuted_by(partconstr,+ list_make1(clause),+ false))+ {+ *contradictory = true;+ return NIL;+ }+ }+ + /*
* Since we only allow strict operators, check for any
* contradicting IS NULL.
*/
@@ -2759,15 +2764,19 @@ get_matching_range_bounds(PartitionPruneContext *context,
* instead as the offset of the upper bound of the greatest
* partition that may contain lookup value. If the lookup
* value had exactly matched the bound, but it isn't
- * inclusive, no need add the adjacent partition. If 'off' is- * -1 indicating that all bounds are greater, then we simply- * end up adding the first bound's offset, that is, 0.+ * inclusive, no need add the adjacent partition.
*/
- else if (off < 0 || !is_equal || inclusive)+ else if (!is_equal || inclusive)
maxoff = off + 1;
else
maxoff = off;
}
+ else+ /*+ * 'off' is -1 indicating that all bounds are greater, so just+ * set the first bound's offset as maxoff.+ */+ maxoff = off + 1;
break;
default:
=
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: hosoya.yuzuko@lab.ntt.co.jp, thibaut.madelaine@dalibo.com, Langote_Amit_f8@lab.ntt.co.jp, imai.yoshikazu@jp.fujitsu.com, pgsql-hackers@lists.postgresql.org
Subject: RE: Problem with default partition pruning
In-Reply-To: <005101d4e074$e24d1e40$a6e75ac0$@lab.ntt.co.jp>
* 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