Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5ac9c43

Browse filesBrowse files
david-rowleydavid-rowley
committed
Cleanup partition pruning step generation
There was some code in gen_prune_steps_from_opexps that needlessly checked a list was not empty when it clearly had to contain at least one item. This prompted a further cleanup operation in partprune.c. Additionally, the previous code could end up adding additional needless INTERSECT steps. However, those do not appear to be able to cause any misbehavior. gen_prune_steps_from_opexps is now no longer in charge of generating combine pruning steps. Instead, gen_partprune_steps_internal, which already does some combine step creation has been given the sole responsibility of generating all combine steps. This means that when we recursively call gen_partprune_steps_internal, since it always now adds a combine step when it produces multiple steps, we can just pay attention to the final step returned. In passing, do quite a bit of work on the comments to try to more clearly explain the role of both gen_partprune_steps_internal and gen_prune_steps_from_opexps. This is fairly complex code so some extra effort to give any new readers an overview of how things work seems like a good idea. Author: Amit Langote Reported-by: Andy Fan Reviewed-by: Kyotaro Horiguchi, Andy Fan, Ryan Lambert, David Rowley Discussion: https://postgr.es/m/CAKU4AWqWoVii+bRTeBQmeVW+PznkdO8DfbwqNsu9Gj4ubt9A6w@mail.gmail.com
1 parent 7e3c541 commit 5ac9c43
Copy full SHA for 5ac9c43

File tree

Expand file treeCollapse file tree

3 files changed

+84
-87
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+84
-87
lines changed

‎src/backend/partitioning/partbounds.c

Copy file name to clipboardExpand all lines: src/backend/partitioning/partbounds.c
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4067,7 +4067,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec)
40674067
if (!list_has_null)
40684068
{
40694069
/*
4070-
* Gin up a "col IS NOT NULL" test that will be AND'd with the main
4070+
* Gin up a "col IS NOT NULL" test that will be ANDed with the main
40714071
* expression. This might seem redundant, but the partition routing
40724072
* machinery needs it.
40734073
*/

‎src/backend/partitioning/partprune.c

Copy file name to clipboardExpand all lines: src/backend/partitioning/partprune.c
+82-85Lines changed: 82 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,8 @@ static PartitionPruneStep *gen_prune_step_op(GeneratePruningStepsContext *contex
156156
static PartitionPruneStep *gen_prune_step_combine(GeneratePruningStepsContext *context,
157157
List *source_stepids,
158158
PartitionPruneCombineOp combineOp);
159-
static PartitionPruneStep *gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
160-
List **keyclauses, Bitmapset *nullkeys);
159+
static List *gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
160+
List **keyclauses, Bitmapset *nullkeys);
161161
static PartClauseMatchStatus match_clause_to_partition_key(GeneratePruningStepsContext *context,
162162
Expr *clause, Expr *partkey, int partkeyidx,
163163
bool *clause_is_not_null,
@@ -924,22 +924,34 @@ get_matching_partitions(PartitionPruneContext *context, List *pruning_steps)
924924

925925
/*
926926
* gen_partprune_steps_internal
927-
* Processes 'clauses' to generate partition pruning steps.
928-
*
929-
* From OpExpr clauses that are mutually AND'd, we find combinations of those
930-
* that match to the partition key columns and for every such combination,
931-
* we emit a PartitionPruneStepOp containing a vector of expressions whose
932-
* values are used as a look up key to search partitions by comparing the
933-
* values with partition bounds. Relevant details of the operator and a
934-
* vector of (possibly cross-type) comparison functions is also included with
935-
* each step.
936-
*
937-
* For BoolExpr clauses, we recursively generate steps for each argument, and
938-
* return a PartitionPruneStepCombine of their results.
939-
*
940-
* The return value is a list of the steps generated, which are also added to
941-
* the context's steps list. Each step is assigned a step identifier, unique
942-
* even across recursive calls.
927+
* Processes 'clauses' to generate a List of partition pruning steps. We
928+
* return NIL when no steps were generated.
929+
*
930+
* These partition pruning steps come in 2 forms; operator steps and combine
931+
* steps.
932+
*
933+
* Operator steps (PartitionPruneStepOp) contain details of clauses that we
934+
* determined that we can use for partition pruning. These contain details of
935+
* the expression which is being compared to the partition key and the
936+
* comparison function.
937+
*
938+
* Combine steps (PartitionPruneStepCombine) instruct the partition pruning
939+
* code how it should produce a single set of partitions from multiple input
940+
* operator and other combine steps. A PARTPRUNE_COMBINE_INTERSECT type
941+
* combine step will merge its input steps to produce a result which only
942+
* contains the partitions which are present in all of the input operator
943+
* steps. A PARTPRUNE_COMBINE_UNION combine step will produce a result that
944+
* has all of the partitions from each of the input operator steps.
945+
*
946+
* For BoolExpr clauses, each argument is processed recursively. Steps
947+
* generated from processing an OR BoolExpr will be combined using
948+
* PARTPRUNE_COMBINE_UNION. AND BoolExprs get combined using
949+
* PARTPRUNE_COMBINE_INTERSECT.
950+
*
951+
* Otherwise, the list of clauses we receive we assume to be mutually ANDed.
952+
* We generate all of the pruning steps we can based on these clauses and then
953+
* at the end, if we have more than 1 step, we combine each step with a
954+
* PARTPRUNE_COMBINE_INTERSECT combine step. Single steps are returned as-is.
943955
*
944956
* If we find clauses that are mutually contradictory, or contradictory with
945957
* the partitioning constraint, or a pseudoconstant clause that contains
@@ -1046,11 +1058,16 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
10461058

10471059
if (argsteps != NIL)
10481060
{
1049-
PartitionPruneStep *step;
1061+
/*
1062+
* gen_partprune_steps_internal() always adds a single
1063+
* combine step when it generates multiple steps, so
1064+
* here we can just pay attention to the last one in
1065+
* the list. If it just generated one, then the last
1066+
* one in the list is still the one we want.
1067+
*/
1068+
PartitionPruneStep *last = llast(argsteps);
10501069

1051-
Assert(list_length(argsteps) == 1);
1052-
step = (PartitionPruneStep *) linitial(argsteps);
1053-
arg_stepids = lappend_int(arg_stepids, step->step_id);
1070+
arg_stepids = lappend_int(arg_stepids, last->step_id);
10541071
}
10551072
else
10561073
{
@@ -1089,9 +1106,7 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
10891106
else if (is_andclause(clause))
10901107
{
10911108
List *args = ((BoolExpr *) clause)->args;
1092-
List *argsteps,
1093-
*arg_stepids = NIL;
1094-
ListCell *lc1;
1109+
List *argsteps;
10951110

10961111
/*
10971112
* args may itself contain clauses of arbitrary type, so just
@@ -1104,21 +1119,16 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
11041119
if (context->contradictory)
11051120
return NIL;
11061121

1107-
foreach(lc1, argsteps)
1108-
{
1109-
PartitionPruneStep *step = lfirst(lc1);
1110-
1111-
arg_stepids = lappend_int(arg_stepids, step->step_id);
1112-
}
1113-
1114-
if (arg_stepids != NIL)
1115-
{
1116-
PartitionPruneStep *step;
1122+
/*
1123+
* gen_partprune_steps_internal() always adds a single combine
1124+
* step when it generates multiple steps, so here we can just
1125+
* pay attention to the last one in the list. If it just
1126+
* generated one, then the last one in the list is still the
1127+
* one we want.
1128+
*/
1129+
if (argsteps != NIL)
1130+
result = lappend(result, llast(argsteps));
11171131

1118-
step = gen_prune_step_combine(context, arg_stepids,
1119-
PARTPRUNE_COMBINE_INTERSECT);
1120-
result = lappend(result, step);
1121-
}
11221132
continue;
11231133
}
11241134

@@ -1253,12 +1263,11 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
12531263
}
12541264
else if (generate_opsteps)
12551265
{
1256-
PartitionPruneStep *step;
1266+
List *opsteps;
12571267

12581268
/* Strategy 2 */
1259-
step = gen_prune_steps_from_opexps(context, keyclauses, nullkeys);
1260-
if (step != NULL)
1261-
result = lappend(result, step);
1269+
opsteps = gen_prune_steps_from_opexps(context, keyclauses, nullkeys);
1270+
result = list_concat(result, opsteps);
12621271
}
12631272
else if (bms_num_members(notnullkeys) == part_scheme->partnatts)
12641273
{
@@ -1271,12 +1280,14 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
12711280
}
12721281

12731282
/*
1274-
* Finally, results from all entries appearing in result should be
1275-
* combined using an INTERSECT combine step, if more than one.
1283+
* Finally, if there are multiple steps, since the 'clauses' are mutually
1284+
* ANDed, add an INTERSECT step to combine the partition sets resulting
1285+
* from them and append it to the result list.
12761286
*/
12771287
if (list_length(result) > 1)
12781288
{
12791289
List *step_ids = NIL;
1290+
PartitionPruneStep *final;
12801291

12811292
foreach(lc, result)
12821293
{
@@ -1285,14 +1296,9 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
12851296
step_ids = lappend_int(step_ids, step->step_id);
12861297
}
12871298

1288-
if (step_ids != NIL)
1289-
{
1290-
PartitionPruneStep *step;
1291-
1292-
step = gen_prune_step_combine(context, step_ids,
1293-
PARTPRUNE_COMBINE_INTERSECT);
1294-
result = lappend(result, step);
1295-
}
1299+
final = gen_prune_step_combine(context, step_ids,
1300+
PARTPRUNE_COMBINE_INTERSECT);
1301+
result = lappend(result, final);
12961302
}
12971303

12981304
return result;
@@ -1356,15 +1362,26 @@ gen_prune_step_combine(GeneratePruningStepsContext *context,
13561362

13571363
/*
13581364
* gen_prune_steps_from_opexps
1359-
* Generate pruning steps based on clauses for partition keys
1360-
*
1361-
* 'keyclauses' contains one list of clauses per partition key. We check here
1362-
* if we have found clauses for a valid subset of the partition key. In some
1363-
* cases, (depending on the type of partitioning being used) if we didn't
1364-
* find clauses for a given key, we discard clauses that may have been
1365-
* found for any subsequent keys; see specific notes below.
1365+
* Generate and return a list of PartitionPruneStepOp that are based on
1366+
* OpExpr and BooleanTest clauses that have been matched to the partition
1367+
* key.
1368+
*
1369+
* 'keyclauses' is an array of List pointers, indexed by the partition key's
1370+
* index. Each List element in the array can contain clauses that match to
1371+
* the corresponding partition key column. Partition key columns without any
1372+
* matched clauses will have an empty List.
1373+
*
1374+
* Some partitioning strategies allow pruning to still occur when we only have
1375+
* clauses for a prefix of the partition key columns, for example, RANGE
1376+
* partitioning. Other strategies, such as HASH partitioning, require clauses
1377+
* for all partition key columns.
1378+
*
1379+
* When we return multiple pruning steps here, it's up to the caller to add a
1380+
* relevant "combine" step to combine the returned steps. This is not done
1381+
* here as callers may wish to include additional pruning steps before
1382+
* combining them all.
13661383
*/
1367-
static PartitionPruneStep *
1384+
static List *
13681385
gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
13691386
List **keyclauses, Bitmapset *nullkeys)
13701387
{
@@ -1397,7 +1414,7 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
13971414
*/
13981415
if (part_scheme->strategy == PARTITION_STRATEGY_HASH &&
13991416
clauselist == NIL && !bms_is_member(i, nullkeys))
1400-
return NULL;
1417+
return NIL;
14011418

14021419
foreach(lc, clauselist)
14031420
{
@@ -1728,27 +1745,7 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
17281745
break;
17291746
}
17301747

1731-
/* Lastly, add a combine step to mutually AND these op steps, if needed */
1732-
if (list_length(opsteps) > 1)
1733-
{
1734-
List *opstep_ids = NIL;
1735-
1736-
foreach(lc, opsteps)
1737-
{
1738-
PartitionPruneStep *step = lfirst(lc);
1739-
1740-
opstep_ids = lappend_int(opstep_ids, step->step_id);
1741-
}
1742-
1743-
if (opstep_ids != NIL)
1744-
return gen_prune_step_combine(context, opstep_ids,
1745-
PARTPRUNE_COMBINE_INTERSECT);
1746-
return NULL;
1747-
}
1748-
else if (opsteps != NIL)
1749-
return linitial(opsteps);
1750-
1751-
return NULL;
1748+
return opsteps;
17521749
}
17531750

17541751
/*
@@ -1782,8 +1779,8 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
17821779
* true otherwise.
17831780
*
17841781
* * PARTCLAUSE_MATCH_STEPS if there is a match.
1785-
* Output arguments: *clause_steps is set to a list of PartitionPruneStep
1786-
* generated for the clause.
1782+
* Output arguments: *clause_steps is set to the list of recursively
1783+
* generated steps for the clause.
17871784
*
17881785
* * PARTCLAUSE_MATCH_CONTRADICT if the clause is self-contradictory, ie
17891786
* it provably returns FALSE or NULL.

‎src/include/nodes/plannodes.h

Copy file name to clipboardExpand all lines: src/include/nodes/plannodes.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1215,7 +1215,7 @@ typedef struct PartitionPruneStep
12151215
} PartitionPruneStep;
12161216

12171217
/*
1218-
* PartitionPruneStepOp - Information to prune using a set of mutually AND'd
1218+
* PartitionPruneStepOp - Information to prune using a set of mutually ANDed
12191219
* OpExpr clauses
12201220
*
12211221
* This contains information extracted from up to partnatts OpExpr clauses,

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.