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
diff --git a/src/backend/partitioning/partprune.c
b/src/backend/partitioning/partprune.c
index 8317318156..aa32a50477 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -795,6 +795,27 @@ gen_partprune_steps_internal(GeneratePruningStepsContext
*context,
Expr *arg = lfirst(lc1);
bool arg_contradictory;
List *argsteps;
+ List *partconstr =
rel->partition_qual;
+
+ /*
+ * If this OR argument 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(arg),
+
false))
+ continue;
+ }
argsteps =
gen_partprune_steps_internal(context, rel,
@@ -822,32 +843,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);
@@ -2759,15 +2761,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:
