From: Yuzuko Hosoya <hosoya.yuzuko@lab.ntt.co.jp>
To: 'Amit Langote' <Langote_Amit_f8@lab.ntt.co.jp>
To: 'Thibaut' <thibaut.madelaine@dalibo.com>
To: 'Imai, Yoshikazu' <imai.yoshikazu@jp.fujitsu.com>
Cc: 'PostgreSQL Hackers' <pgsql-hackers@lists.postgresql.org>
Subject: RE: Problem with default partition pruning
Date: Mon, 8 Apr 2019 16:57:35 +0900
Message-ID: <00c101d4ede0$babd4390$3037cab0$@lab.ntt.co.jp> (raw)
In-Reply-To: <9dc22159-4869-dd2b-feb5-c39af3c01a7e@lab.ntt.co.jp>
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>
<005101d4e074$e24d1e40$a6e75ac0$@lab.ntt.co.jp>
<001ea23e-638f-9ab2-143d-2cf1105bf8d6@lab.ntt.co.jp>
<008f01d4e911$39aa87a0$acff96e0$@lab.ntt.co.jp>
<c677422b-b120-a622-fdff-44bd1158b4fe@lab.ntt.co.jp>
<00a301d4ea9b$01296eb0$037c4c10$@lab.ntt.co.jp>
<9dc22159-4869-dd2b-feb5-c39af3c01a7e@lab.ntt.co.jp>
Amit-san,
> -----Original Message-----
> From: Amit Langote [mailto:Langote_Amit_f8@lab.ntt.co.jp]
> Sent: Friday, April 05, 2019 6:47 PM
> To: Yuzuko Hosoya <hosoya.yuzuko@lab.ntt.co.jp>; 'Thibaut' <thibaut.madelaine@dalibo.com>; 'Imai,
> Yoshikazu' <imai.yoshikazu@jp.fujitsu.com>
> Cc: 'PostgreSQL Hackers' <pgsql-hackers@lists.postgresql.org>
> Subject: Re: Problem with default partition pruning
>
> Hosoya-san,
>
>
> On 2019/04/04 13:00, Yuzuko Hosoya wrote:
> > I added some test cases to each patch according to tests discussed in
> > this thread.
>
> Thanks a lot.
>
> > However, I found another problem as follows. This query should output
> > "One-Time Filter: false" because rlp3's constraints contradict WHERE
> > clause.
> >
> > -----
> > postgres=# \d+ rlp3
> > Partitioned table "public.rlp3"
> > Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
> >
> --------+-------------------+-----------+----------+---------+----------+--------------+---------
> ----
> > b | character varying | | | | extended | |
> > a | integer | | | | plain | |
> > Partition of: rlp FOR VALUES FROM (15) TO (20) Partition constraint:
> > ((a IS NOT NULL) AND (a >= 15) AND (a < 20)) Partition key: LIST (b
> > varchar_ops)
> > Partitions: rlp3abcd FOR VALUES IN ('ab', 'cd'),
> > rlp3efgh FOR VALUES IN ('ef', 'gh'),
> > rlp3nullxy FOR VALUES IN (NULL, 'xy'),
> > rlp3_default DEFAULT
> >
> > postgres=# explain select * from rlp3 where a = 2;
> > QUERY PLAN
> > --------------------------------------------------------------------
> > Append (cost=0.00..103.62 rows=24 width=36)
> > -> Seq Scan on rlp3abcd (cost=0.00..25.88 rows=6 width=36)
> > Filter: (a = 2)
> > -> Seq Scan on rlp3efgh (cost=0.00..25.88 rows=6 width=36)
> > Filter: (a = 2)
> > -> Seq Scan on rlp3nullxy (cost=0.00..25.88 rows=6 width=36)
> > Filter: (a = 2)
> > -> Seq Scan on rlp3_default (cost=0.00..25.88 rows=6 width=36)
> > Filter: (a = 2)
> > (9 rows)
> > -----
>
> This one too would be solved with the other patch I mentioned to fix
> get_relation_info() to load the partition constraint so that constraint exclusion can use it.
> Partition in the earlier example given by Thibaut is a leaf partition, whereas rlp3 above is a
> sub-partitioned partition, but both are partitions nonetheless.
>
> Fixing partprune.c like we're doing with the
> v2_ignore_contradictory_where_clauses_at_partprune_step.patch only works for the latter, because only
> partitioned tables visit partprune.c.
>
> OTOH, the other patch only applies to situations where constraint_exclusion = on.
>
I see. I think that following example discussed in this thread before would
also be solved with your patch, not v2_ignore_contradictory_where_clauses_at_partprune_step.patch.
postgres=# set constraint_exclusion to on;
postgres=# explain select * from test2_0_20 where id = 25;
QUERY PLAN
------------------------------------------
Result (cost=0.00..0.00 rows=0 width=0)
One-Time Filter: false
(2 rows)
> > I think that the place of check contradiction process was wrong At
> > ignore_contradictory_where_clauses_at_partprune_step.patch.
> > So I fixed it.
>
> Thanks. Patch contains some whitespace noise:
>
> $ git diff --check
> src/backend/partitioning/partprune.c:790: trailing whitespace.
> + * given its partition constraint, we can ignore it,
> src/backend/partitioning/partprune.c:791: trailing whitespace.
> + * that is not try to pass it to the pruning code.
> src/backend/partitioning/partprune.c:792: trailing whitespace.
> + * We should do that especially to avoid pruning code
> src/backend/partitioning/partprune.c:810: trailing whitespace.
> +
> src/test/regress/sql/partition_prune.sql:87: trailing whitespace.
> +-- where clause contradicts sub-partition's constraint
>
> Can you please fix it?
>
Thanks for checking.
I'm attaching the latest patch.
>
> BTW, now I'm a bit puzzled between whether this case should be fixed by hacking on partprune.c like
> this patch does or whether to work on getting the other patch committed and expect users to set
> constraint_exclusion = on for this to behave as expected. The original intention of setting
> partition_qual in set_relation_partition_info() was for partprune.c to use it to remove useless
> arguments of OR clauses which otherwise would cause the failure to correctly prune the default partitions
> of sub-partitioned tables. As shown by the examples in this thread, the original effort was
> insufficient, which this patch aims to improve. But, it also expands the scope of partprune.c's usage
> of partition_qual, which is to effectively perform full-blown constraint exclusion without being
> controllable by constraint_exclusion GUC, which may be seen as being good or bad. The fact that it
> helps in getting partition pruning working correctly in more obscure cases like those discussed in
> this thread means it's good maybe.
>
Umm, even though this modification might be overhead, I think this problem should be solved
without setting constraint_exclusion GUC. But I'm not sure.
Best regards,
Yuzuko Hosoya
Attachments:
[application/octet-stream] v3_ignore_contradictory_where_clauses_at_partprune_step.patch (4.3K, ../00c101d4ede0$babd4390$3037cab0$@lab.ntt.co.jp/2-v3_ignore_contradictory_where_clauses_at_partprune_step.patch)
download | inline diff:diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.cindex 5cd740a..f360d82 100644--- a/src/backend/partitioning/partprune.c+++ b/src/backend/partitioning/partprune.c@@ -783,11 +783,35 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
{
Expr *clause = (Expr *) lfirst(lc);
int i;
+ List *partconstr = rel->partition_qual;
/* Look through RestrictInfo, if any */
if (IsA(clause, RestrictInfo))
clause = ((RestrictInfo *) clause)->clause;
+ /*+ * 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;+ }+ }+
/* Constant-false-or-null is contradictory */
if (IsA(clause, Const) &&
(((Const *) clause)->constisnull ||
@@ -850,32 +874,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);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.outindex b46951c..716155d 100644--- a/src/test/regress/expected/partition_prune.out+++ b/src/test/regress/expected/partition_prune.out@@ -592,6 +592,24 @@ explain (costs off) select * from rlp where a < 1 or (a > 20 and a < 25);
Filter: ((a < 1) OR ((a > 20) AND (a < 25)))
(5 rows)
+-- where clause contradicts sub-partition's constraint +explain (costs off) select * from rlp where a = 20 or a = 40;+ QUERY PLAN +----------------------------------------+ Append+ -> Seq Scan on rlp4_1+ Filter: ((a = 20) OR (a = 40))+ -> Seq Scan on rlp5_default+ Filter: ((a = 20) OR (a = 40))+(5 rows)++explain (costs off) select * from rlp3 where a = 20; /* empty */+ QUERY PLAN +--------------------------+ Result+ One-Time Filter: false+(2 rows)+
-- redundant clauses are eliminated
explain (costs off) select * from rlp where a > 1 and a = 10; /* only default */
QUERY PLAN
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sqlindex abc6d9e..a5e8109 100644--- a/src/test/regress/sql/partition_prune.sql+++ b/src/test/regress/sql/partition_prune.sql@@ -85,6 +85,10 @@ explain (costs off) select * from rlp where a = 29;
explain (costs off) select * from rlp where a >= 29;
explain (costs off) select * from rlp where a < 1 or (a > 20 and a < 25);
+-- where clause contradicts sub-partition's constraint+explain (costs off) select * from rlp where a = 20 or a = 40;+explain (costs off) select * from rlp3 where a = 20; /* empty */+
-- redundant clauses are eliminated
explain (costs off) select * from rlp where a > 1 and a = 10; /* only default */
explain (costs off) select * from rlp where a > 1 and a >=15; /* rlp3 onwards, including 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, Langote_Amit_f8@lab.ntt.co.jp, thibaut.madelaine@dalibo.com, imai.yoshikazu@jp.fujitsu.com, pgsql-hackers@lists.postgresql.org
Subject: RE: Problem with default partition pruning
In-Reply-To: <00c101d4ede0$babd4390$3037cab0$@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