On 2018-Jul-13, Ashutosh Bapat wrote:
> On Fri, Jul 13, 2018 at 1:15 PM, Amit Langote
> <[email protected]> wrote:
> >>
> >> I don't think this is true. When equality conditions and IS NULL clauses
> >> cover
> >> all partition keys of a hash partitioned table and do not have
> >> contradictory
> >> clauses, we should be able to find the partition which will remain
> >> unpruned.
> >
> > I was trying to say the same thing, but maybe the comment doesn't like it.
> > How about this:
> >
> > + * For hash partitioning, if we found IS NULL clauses for some keys and
> > + * OpExpr's for others, gen_prune_steps_from_opexps() will generate the
> > + * necessary pruning steps if all partition keys are taken care of.
> > + * If we found only IS NULL clauses, then we can generate the pruning
> > + * step here but only if all partition keys are covered.
> >
>
> It's still confusing a bit. I think "taken care of" doesn't convey the
> same meaning as "covered". I don't think the second sentence is
> necessary, it talks about one special case of the first sentence. But
> this is better than the prior version.
Hmm, let me reword this comment completely. How about the attached?
I also changed the order of the tests, putting the 'generate_opsteps'
one in the middle instead of at the end (so the not-null one doesn't
test that boolean again). AFAICS it's logically the same, and it makes
more sense to me that way.
I also changed the tests so that they apply to the existing mc2p table,
which is identical to the one Amit was adding.
> > How about if we explicitly spell out the strategies like this:
> >
> > + if (!bms_is_empty(nullkeys) &&
> > + (part_scheme->strategy == PARTITION_STRATEGY_LIST ||
> > + part_scheme->strategy == PARTITION_STRATEGY_RANGE ||
> > + (part_scheme->strategy == PARTITION_STRATEGY_HASH &&
> > + bms_num_members(nullkeys) == part_scheme->partnatts)))
>
> I still do not understand why do we need (part_scheme->strategy ==
> PARTITION_STRATEGY_HASH && bms_num_members(nullkeys) ==
> part_scheme->partnatts) there. I thought that this will be handled by
> code, which deals with null keys and op_exprs together, somewhere
> else.
I'm not sure I understand this question. This strategy applies to hash
partitioning only if we have null tests for all keys, so there are no
op_exprs. If there are any, there's no point in checking them.
Now this case is a fun one:
select * from mc2p where a in (1, 2) and b is null;
Note that no partitions are pruned in that case; yet if you do this:
select * from mc2p where a in (1) and b is null;
then it does prune. I think the reason for this is that the
PARTCLAUSE_MATCH_STEPS is ... pretty special.
> > Actually, there is only one case where the pruning step generated by that
> > block of code is useful -- to prune a list partition that accepts only
> > nulls. List partitioning only allows one partition, key so this works,
> > but let's say only accidentally. I modified the condition as follows:
> >
> > + else if (!generate_opsteps && !bms_is_empty(notnullkeys) &&
> > + bms_num_members(notnullkeys) == part_scheme->partnatts)
> >
>
> Hmm. Ok. May be it's better to explicitly say that it's only useful in
> case of list partitioned table.
Yeah, maybe.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/partitioning/partprune.c
b/src/backend/partitioning/partprune.c
index cdc61a8997..e44df8ac94 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -853,54 +853,62 @@ gen_partprune_steps_internal(GeneratePruningStepsContext
*context,
}
}
- /*
- * If generate_opsteps is set to false it means no OpExprs were directly
- * present in the input list.
+ /*-----------
+ * Now generate some (more) pruning steps.
+ *
+ * We may be able to (1) generate pruning steps based on IS NULL
clauses,
+ * if there are enough of them:
+ * a) For list partitioning, null partition keys can only be found in
the
+ * designated partition, so if there are IS NULL clauses containing
+ * partition keys we should generate a pruning step that gets rid of
+ * all partitions but the special null-accepting partitions.
+ * b) For range partitioning, the same applies. Doing this check first
+ * means we'll disregard OpExprs that may have been found for other
+ * keys, but that's fine, because it is not possible to prune range
+ * partitions with a combination of null and non-null keys.
+ * c) For hash partitioning, we only apply this strategy if we have
+ * IS NULL clauses for all the keys. Strategy 2 will take care of
the
+ * case where some keys have OpExprs and others have IS NULL clauses.
+ *
+ * If not, then (2) generate steps based on OpExprs we have (if any).
+ *
+ * If this doesn't work either, we may be able to (3) generate steps to
+ * prune (just) the null-accepting partition (if one exists), if we have
+ * IS NOT NULL clauses for all partition keys.
*/
- if (!generate_opsteps)
- {
- /*
- * Generate one prune step for the information derived from IS
NULL,
- * if any. To prune hash partitions, we must have found IS NULL
- * clauses for all partition keys.
- */
- if (!bms_is_empty(nullkeys) &&
- (part_scheme->strategy != PARTITION_STRATEGY_HASH ||
- bms_num_members(nullkeys) == part_scheme->partnatts))
- {
- PartitionPruneStep *step;
-
- step = gen_prune_step_op(context, InvalidStrategy,
- false,
NIL, NIL, nullkeys);
- result = lappend(result, step);
- }
-
- /*
- * Note that for IS NOT NULL clauses, simply having step
suffices;
- * there is no need to propagate the exact details of which
keys are
- * required to be NOT NULL. Hash partitioning expects to see
actual
- * values to perform any pruning.
- */
- if (!bms_is_empty(notnullkeys) &&
- part_scheme->strategy != PARTITION_STRATEGY_HASH)
- {
- PartitionPruneStep *step;
-
- step = gen_prune_step_op(context, InvalidStrategy,
- false,
NIL, NIL, NULL);
- result = lappend(result, step);
- }
- }
- else
+ if (!bms_is_empty(nullkeys) &&
+ (part_scheme->strategy == PARTITION_STRATEGY_LIST ||
+ part_scheme->strategy == PARTITION_STRATEGY_RANGE ||
+ (part_scheme->strategy == PARTITION_STRATEGY_HASH &&
+ bms_num_members(nullkeys) == part_scheme->partnatts)))
{
PartitionPruneStep *step;
- /* Generate pruning steps from OpExpr clauses in keyclauses. */
+ /* Strategy 1 */
+ step = gen_prune_step_op(context, InvalidStrategy,
+ false, NIL,
NIL, nullkeys);
+ result = lappend(result, step);
+ }
+ else if (generate_opsteps)
+ {
+ PartitionPruneStep *step;
+
+ /* Strategy 2 */
step = gen_prune_steps_from_opexps(part_scheme, context,
keyclauses, nullkeys);
if (step != NULL)
result = lappend(result, step);
}
+ else if (!bms_is_empty(notnullkeys) &&
+ bms_num_members(notnullkeys) == part_scheme->partnatts)
+ {
+ PartitionPruneStep *step;
+
+ /* Strategy 3 */
+ step = gen_prune_step_op(context, InvalidStrategy,
+ false, NIL,
NIL, NULL);
+ result = lappend(result, step);
+ }
/*
* Finally, results from all entries appearing in result should be
diff --git a/src/test/regress/expected/partition_prune.out
b/src/test/regress/expected/partition_prune.out
index d15f1d37f1..022b7c55c7 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -993,6 +993,47 @@ explain (costs off) select * from mc2p where a = 1 and b >
1;
Filter: ((b > 1) AND (a = 1))
(3 rows)
+-- all partitions but the default one should be pruned
+explain (costs off) select * from mc2p where a = 1 and b is null;
+ QUERY PLAN
+-------------------------------------------
+ Append
+ -> Seq Scan on mc2p_default
+ Filter: ((b IS NULL) AND (a = 1))
+(3 rows)
+
+explain (costs off) select * from mc2p where a is null and b is null;
+ QUERY PLAN
+-----------------------------------------------
+ Append
+ -> Seq Scan on mc2p_default
+ Filter: ((a IS NULL) AND (b IS NULL))
+(3 rows)
+
+explain (costs off) select * from mc2p where a is null and b = 1;
+ QUERY PLAN
+-------------------------------------------
+ Append
+ -> Seq Scan on mc2p_default
+ Filter: ((a IS NULL) AND (b = 1))
+(3 rows)
+
+explain (costs off) select * from mc2p where a is null;
+ QUERY PLAN
+--------------------------------
+ Append
+ -> Seq Scan on mc2p_default
+ Filter: (a IS NULL)
+(3 rows)
+
+explain (costs off) select * from mc2p where b is null;
+ QUERY PLAN
+--------------------------------
+ Append
+ -> Seq Scan on mc2p_default
+ Filter: (b IS NULL)
+(3 rows)
+
-- boolean partitioning
create table boolpart (a bool) partition by list (a);
create table boolpart_default partition of boolpart default;
diff --git a/src/test/regress/sql/partition_prune.sql
b/src/test/regress/sql/partition_prune.sql
index b8e823d562..2357f02cde 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -137,6 +137,13 @@ explain (costs off) select * from mc2p where a = 2 and b <
1;
explain (costs off) select * from mc2p where a > 1;
explain (costs off) select * from mc2p where a = 1 and b > 1;
+-- all partitions but the default one should be pruned
+explain (costs off) select * from mc2p where a = 1 and b is null;
+explain (costs off) select * from mc2p where a is null and b is null;
+explain (costs off) select * from mc2p where a is null and b = 1;
+explain (costs off) select * from mc2p where a is null;
+explain (costs off) select * from mc2p where b is null;
+
-- boolean partitioning
create table boolpart (a bool) partition by list (a);
create table boolpart_default partition of boolpart default;
