From b18d1bd8d0cbd834c0eb9dc20a0883800508fe3f Mon Sep 17 00:00:00 2001 From: "dgrowley@gmail.com" Date: Wed, 7 Apr 2021 23:19:08 +1200 Subject: [PATCH v3] Cosmetic improvements to partition pruning step generation code Make gen_partprune_steps_from_opexprs() return the "op" steps (PartitionPruneOpStep) it generates in the list instead of a single "combine" step that combines their result. The new behavior better matches its name. Instead make it the job of its higher level caller gen_partprune_steps_internal() to add the "combine" step if needed. To match, also change its return type to be PartitionPruneStep * rather than List *. This also makes the job of its recursive callers a bit simpler, because they either expect a single step to be returned or combine the multiple steps into a single step anyway. While at it, also fix some comments. --- src/backend/partitioning/partbounds.c | 2 +- src/backend/partitioning/partprune.c | 219 ++++++++++++-------------- src/include/nodes/plannodes.h | 2 +- 3 files changed, 102 insertions(+), 121 deletions(-) diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 2b2b1dc1ad..bfa6e27e9b 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -4067,7 +4067,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec) if (!list_has_null) { /* - * Gin up a "col IS NOT NULL" test that will be AND'd with the main + * Gin up a "col IS NOT NULL" test that will be ANDed with the main * expression. This might seem redundant, but the partition routing * machinery needs it. */ diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c index d08739127b..5ab00cdcf1 100644 --- a/src/backend/partitioning/partprune.c +++ b/src/backend/partitioning/partprune.c @@ -148,7 +148,7 @@ static List *make_partitionedrel_pruneinfo(PlannerInfo *root, static void gen_partprune_steps(RelOptInfo *rel, List *clauses, PartClauseTarget target, GeneratePruningStepsContext *context); -static List *gen_partprune_steps_internal(GeneratePruningStepsContext *context, +static PartitionPruneStep *gen_partprune_steps_internal(GeneratePruningStepsContext *context, List *clauses); static PartitionPruneStep *gen_prune_step_op(GeneratePruningStepsContext *context, StrategyNumber opstrategy, bool op_is_ne, @@ -156,12 +156,13 @@ static PartitionPruneStep *gen_prune_step_op(GeneratePruningStepsContext *contex static PartitionPruneStep *gen_prune_step_combine(GeneratePruningStepsContext *context, List *source_stepids, PartitionPruneCombineOp combineOp); -static PartitionPruneStep *gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, - List **keyclauses, Bitmapset *nullkeys); +static List *gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, + List **keyclauses, Bitmapset *nullkeys); static PartClauseMatchStatus match_clause_to_partition_key(GeneratePruningStepsContext *context, Expr *clause, Expr *partkey, int partkeyidx, bool *clause_is_not_null, - PartClauseInfo **pc, List **clause_steps); + PartClauseInfo **pc, + PartitionPruneStep **clause_step); static List *get_steps_using_prefix(GeneratePruningStepsContext *context, StrategyNumber step_opstrategy, bool step_op_is_ne, @@ -924,30 +925,39 @@ get_matching_partitions(PartitionPruneContext *context, List *pruning_steps) /* * gen_partprune_steps_internal - * Processes 'clauses' to generate partition pruning steps. - * - * From OpExpr clauses that are mutually AND'd, we find combinations of those - * that match to the partition key columns and for every such combination, - * we emit a PartitionPruneStepOp containing a vector of expressions whose - * values are used as a look up key to search partitions by comparing the - * values with partition bounds. Relevant details of the operator and a - * vector of (possibly cross-type) comparison functions is also included with - * each step. - * - * For BoolExpr clauses, we recursively generate steps for each argument, and - * return a PartitionPruneStepCombine of their results. - * - * The return value is a list of the steps generated, which are also added to - * the context's steps list. Each step is assigned a step identifier, unique - * even across recursive calls. + * Processes 'clauses' to generate a set of partition pruning steps. We + * return a single PartitionPruneStep, which may be a single step or + * several steps combined into either a PARTPRUNE_COMBINE_INTERSECT or + * PARTPRUNE_COMBINE_UNION step. We return NULL when no steps were + * generated. + * + * Combine steps instruct the partition pruning code how it should produce a + * single set of partitions from multiple input pruning steps. A + * PARTPRUNE_COMBINE_INTERSECT type combine step will merge its input steps + * to produce a result which only contains the partitions which are present in + * all of the input steps. A PARTPRUNE_COMBINE_UNION combine step will + * produce a result that has all of the partitions from each of the input + * steps. + * + * From OpExpr clauses that are mutually ANDed, we find combinations of those + * that match the partition key columns and make one or more "op" steps + * (PartitionPruneStepOp) from those clause combinations. + * + * For BoolExpr clauses, each argument is processed recursively. Steps + * generated from processing an OR BoolExpr will be combined using + * PARTPRUNE_COMBINE_UNION. AND BoolExprs get combined using + * PARTPRUNE_COMBINE_INTERSECT. + * + * The return value in the top-level call is the final step whose evaluation + * will give the partition set satisfying the provided clauses. * * If we find clauses that are mutually contradictory, or contradictory with * the partitioning constraint, or a pseudoconstant clause that contains - * false, we set context->contradictory to true and return NIL (that is, no - * pruning steps). Caller should consider all partitions as pruned in that + * false, we set context->contradictory to true and return NULL (that is, no + * pruning steps). The caller should consider all partitions as pruned in that * case. */ -static List * +static PartitionPruneStep * gen_partprune_steps_internal(GeneratePruningStepsContext *context, List *clauses) { @@ -956,7 +966,7 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, Bitmapset *nullkeys = NULL, *notnullkeys = NULL; bool generate_opsteps = false; - List *result = NIL; + List *steps = NIL; ListCell *lc; /* @@ -975,7 +985,7 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, predicate_refuted_by(context->rel->partition_qual, clauses, false)) { context->contradictory = true; - return NIL; + return NULL; } memset(keyclauses, 0, sizeof(keyclauses)); @@ -994,7 +1004,7 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, !DatumGetBool(((Const *) clause)->constvalue))) { context->contradictory = true; - return NIL; + return NULL; } /* Get the BoolExpr's out of the way. */ @@ -1028,10 +1038,10 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, { Expr *arg = lfirst(lc1); bool arg_contradictory; - List *argsteps; + PartitionPruneStep *step; - argsteps = gen_partprune_steps_internal(context, - list_make1(arg)); + step = gen_partprune_steps_internal(context, + list_make1(arg)); arg_contradictory = context->contradictory; /* Keep context->contradictory clear till we're done */ context->contradictory = false; @@ -1044,14 +1054,8 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, else all_args_contradictory = false; - if (argsteps != NIL) - { - PartitionPruneStep *step; - - Assert(list_length(argsteps) == 1); - step = (PartitionPruneStep *) linitial(argsteps); + if (step != NULL) arg_stepids = lappend_int(arg_stepids, step->step_id); - } else { PartitionPruneStep *orstep; @@ -1073,7 +1077,7 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, if (all_args_contradictory) { context->contradictory = true; - return NIL; + return NULL; } if (arg_stepids != NIL) @@ -1082,43 +1086,28 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, step = gen_prune_step_combine(context, arg_stepids, PARTPRUNE_COMBINE_UNION); - result = lappend(result, step); + steps = lappend(steps, step); } continue; } else if (is_andclause(clause)) { List *args = ((BoolExpr *) clause)->args; - List *argsteps, - *arg_stepids = NIL; - ListCell *lc1; + PartitionPruneStep *step; /* * args may itself contain clauses of arbitrary type, so just * recurse and later combine the component partitions sets * using a combine step. */ - argsteps = gen_partprune_steps_internal(context, args); + step = gen_partprune_steps_internal(context, args); /* If any AND arm is contradictory, we can stop immediately */ if (context->contradictory) - return NIL; + return NULL; - foreach(lc1, argsteps) - { - PartitionPruneStep *step = lfirst(lc1); + steps = lappend(steps, step); - arg_stepids = lappend_int(arg_stepids, step->step_id); - } - - if (arg_stepids != NIL) - { - PartitionPruneStep *step; - - step = gen_prune_step_combine(context, arg_stepids, - PARTPRUNE_COMBINE_INTERSECT); - result = lappend(result, step); - } continue; } @@ -1138,12 +1127,12 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, Expr *partkey = linitial(context->rel->partexprs[i]); bool clause_is_not_null = false; PartClauseInfo *pc = NULL; - List *clause_steps = NIL; + PartitionPruneStep *clause_step = NULL; switch (match_clause_to_partition_key(context, clause, partkey, i, &clause_is_not_null, - &pc, &clause_steps)) + &pc, &clause_step)) { case PARTCLAUSE_MATCH_CLAUSE: Assert(pc != NULL); @@ -1155,7 +1144,7 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, if (bms_is_member(i, nullkeys)) { context->contradictory = true; - return NIL; + return NULL; } generate_opsteps = true; keyclauses[i] = lappend(keyclauses[i], pc); @@ -1172,7 +1161,7 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, keyclauses[i] != NIL) { context->contradictory = true; - return NIL; + return NULL; } nullkeys = bms_add_member(nullkeys, i); } @@ -1182,21 +1171,21 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, if (bms_is_member(i, nullkeys)) { context->contradictory = true; - return NIL; + return NULL; } notnullkeys = bms_add_member(notnullkeys, i); } break; case PARTCLAUSE_MATCH_STEPS: - Assert(clause_steps != NIL); - result = list_concat(result, clause_steps); + Assert(clause_step != NULL); + steps = lappend(steps, clause_step); break; case PARTCLAUSE_MATCH_CONTRADICT: /* We've nothing more to do if a contradiction was found. */ context->contradictory = true; - return NIL; + return NULL; case PARTCLAUSE_NOMATCH: @@ -1249,16 +1238,15 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, /* Strategy 1 */ step = gen_prune_step_op(context, InvalidStrategy, false, NIL, NIL, nullkeys); - result = lappend(result, step); + steps = lappend(steps, step); } else if (generate_opsteps) { - PartitionPruneStep *step; + List *opsteps; /* Strategy 2 */ - step = gen_prune_steps_from_opexps(context, keyclauses, nullkeys); - if (step != NULL) - result = lappend(result, step); + opsteps = gen_prune_steps_from_opexps(context, keyclauses, nullkeys); + steps = list_concat(steps, opsteps); } else if (bms_num_members(notnullkeys) == part_scheme->partnatts) { @@ -1267,35 +1255,37 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, /* Strategy 3 */ step = gen_prune_step_op(context, InvalidStrategy, false, NIL, NIL, NULL); - result = lappend(result, step); + steps = lappend(steps, step); } + /* No pruning possible without any steps */ + if (steps == NIL) + return NULL; + /* - * Finally, results from all entries appearing in result should be - * combined using an INTERSECT combine step, if more than one. + * Finally, if there are multiple steps, combine these using an INTERSECT + * step and return that. */ - if (list_length(result) > 1) + if (list_length(steps) > 1) { List *step_ids = NIL; - foreach(lc, result) + foreach(lc, steps) { PartitionPruneStep *step = lfirst(lc); step_ids = lappend_int(step_ids, step->step_id); } - if (step_ids != NIL) - { - PartitionPruneStep *step; - - step = gen_prune_step_combine(context, step_ids, - PARTPRUNE_COMBINE_INTERSECT); - result = lappend(result, step); - } + return gen_prune_step_combine(context, step_ids, + PARTPRUNE_COMBINE_INTERSECT); } - return result; + /* + * steps contains just a single step. There's no sense in adding a + * combine on a single step, so just return it as-is. + */ + return linitial(steps); } /* @@ -1356,15 +1346,26 @@ gen_prune_step_combine(GeneratePruningStepsContext *context, /* * gen_prune_steps_from_opexps - * Generate pruning steps based on clauses for partition keys - * - * 'keyclauses' contains one list of clauses per partition key. We check here - * if we have found clauses for a valid subset of the partition key. In some - * cases, (depending on the type of partitioning being used) if we didn't - * find clauses for a given key, we discard clauses that may have been - * found for any subsequent keys; see specific notes below. + * Generate and return a list of PartitionPruneStepOp that are based on + * OpExpr and BooleanTest clauses that have been matched to the partition + * key. + * + * 'keyclauses' is an array of List pointers, indexed by the partition key's + * index. Each List element in the array can contain clauses that match to + * the corresponding partition key column. Partition key columns without any + * matched clauses will have an empty List. + * + * Some partitioning strategies allow pruning to still occur when we only have + * clauses for a prefix of the partition key columns, for example, RANGE + * partitioning. Other strategies, such as HASH partitioning, require clauses + * for all partition key columns. + * + * When we return multiple pruning steps here, it's up to the caller to add a + * relevant "combine" step to combine the returned steps. This is not done + * here as callers may wish to include additional pruning steps before + * combining them all. */ -static PartitionPruneStep * +static List * gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, List **keyclauses, Bitmapset *nullkeys) { @@ -1397,7 +1398,7 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, */ if (part_scheme->strategy == PARTITION_STRATEGY_HASH && clauselist == NIL && !bms_is_member(i, nullkeys)) - return NULL; + return NIL; foreach(lc, clauselist) { @@ -1728,27 +1729,7 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, break; } - /* Lastly, add a combine step to mutually AND these op steps, if needed */ - if (list_length(opsteps) > 1) - { - List *opstep_ids = NIL; - - foreach(lc, opsteps) - { - PartitionPruneStep *step = lfirst(lc); - - opstep_ids = lappend_int(opstep_ids, step->step_id); - } - - if (opstep_ids != NIL) - return gen_prune_step_combine(context, opstep_ids, - PARTPRUNE_COMBINE_INTERSECT); - return NULL; - } - else if (opsteps != NIL) - return linitial(opsteps); - - return NULL; + return opsteps; } /* @@ -1782,8 +1763,8 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, * true otherwise. * * * PARTCLAUSE_MATCH_STEPS if there is a match. - * Output arguments: *clause_steps is set to a list of PartitionPruneStep - * generated for the clause. + * Output arguments: *clause_step is set to the PartitionPruneStep generated + * for the clause. * * * PARTCLAUSE_MATCH_CONTRADICT if the clause is self-contradictory, ie * it provably returns FALSE or NULL. @@ -1798,7 +1779,7 @@ static PartClauseMatchStatus match_clause_to_partition_key(GeneratePruningStepsContext *context, Expr *clause, Expr *partkey, int partkeyidx, bool *clause_is_not_null, PartClauseInfo **pc, - List **clause_steps) + PartitionPruneStep **clause_step) { PartClauseMatchStatus boolmatchstatus; PartitionScheme part_scheme = context->rel->part_scheme; @@ -2309,10 +2290,10 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context, elem_clauses = list_make1(makeBoolExpr(OR_EXPR, elem_clauses, -1)); /* Finally, generate steps */ - *clause_steps = gen_partprune_steps_internal(context, elem_clauses); + *clause_step = gen_partprune_steps_internal(context, elem_clauses); if (context->contradictory) return PARTCLAUSE_MATCH_CONTRADICT; - else if (*clause_steps == NIL) + else if (*clause_step == NULL) return PARTCLAUSE_UNSUPPORTED; /* step generation failed */ return PARTCLAUSE_MATCH_STEPS; } diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 1678bd66fe..d671328dfd 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1215,7 +1215,7 @@ typedef struct PartitionPruneStep } PartitionPruneStep; /* - * PartitionPruneStepOp - Information to prune using a set of mutually AND'd + * PartitionPruneStepOp - Information to prune using a set of mutually ANDed * OpExpr clauses * * This contains information extracted from up to partnatts OpExpr clauses, -- 2.27.0