Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commitcd259de

Browse files
committed
Fix incorrect step generation in HASH partition pruning
get_steps_using_prefix_recurse() incorrectly assumed that it could stoprecursive processing of the 'prefix' list when cur_keyno was one beforethe step_lastkeyno. Since hash partition pruning can prune using ISNULL quals, and these IS NULL quals are not present in the 'prefix'list, then that logic could cause more levels of recursion than what isneeded and lead to there being no more items in the 'prefix' list toprocess. This would manifest itself as a crash in some code thatexpected the 'start' ListCell not to be NULL.Here we adjust the logic so that instead of stopping recursion at 1 keybefore the step_lastkeyno, we just look at the llast(prefix) item andensure we only recursively process up until just before whichever the lastkey is. This effectively allows keys to be missing in the 'prefix' list.This change does mean that step_lastkeyno is no longer needed, so weremove that from the static functions. I also spent quite some timereading this code and testing it to try to convince myself that thereare no other issues. That resulted in the irresistible temptation ofrewriting some comments, many of which were just not true or inconcise.Reported-by: Sergei GlukhovReviewed-by: Sergei Glukhov, tender wangDiscussion:https://postgr.es/m/2f09ce72-315e-2a33-589a-8519ada8df61@postgrespro.ruBackpatch-through: 11, where partition pruning was introduced.
1 parent077cedf commitcd259de

File tree

3 files changed

+319
-61
lines changed

3 files changed

+319
-61
lines changed

‎src/backend/partitioning/partprune.c

Lines changed: 63 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -164,16 +164,15 @@ static List *get_steps_using_prefix(GeneratePruningStepsContext *context,
164164
boolstep_op_is_ne,
165165
Expr*step_lastexpr,
166166
Oidstep_lastcmpfn,
167-
intstep_lastkeyno,
168167
Bitmapset*step_nullkeys,
169168
List*prefix);
170169
staticList*get_steps_using_prefix_recurse(GeneratePruningStepsContext*context,
171170
StrategyNumberstep_opstrategy,
172171
boolstep_op_is_ne,
173172
Expr*step_lastexpr,
174173
Oidstep_lastcmpfn,
175-
intstep_lastkeyno,
176174
Bitmapset*step_nullkeys,
175+
List*prefix,
177176
ListCell*start,
178177
List*step_exprs,
179178
List*step_cmpfns);
@@ -1424,7 +1423,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
14241423
pc->op_is_ne,
14251424
pc->expr,
14261425
pc->cmpfn,
1427-
0,
14281426
NULL,
14291427
NIL);
14301428
opsteps=list_concat(opsteps,pc_steps);
@@ -1550,7 +1548,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
15501548
pc->op_is_ne,
15511549
pc->expr,
15521550
pc->cmpfn,
1553-
pc->keyno,
15541551
NULL,
15551552
prefix);
15561553
opsteps=list_concat(opsteps,pc_steps);
@@ -1624,7 +1621,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
16241621
false,
16251622
pc->expr,
16261623
pc->cmpfn,
1627-
pc->keyno,
16281624
nullkeys,
16291625
prefix);
16301626
opsteps=list_concat(opsteps,list_copy(pc_steps));
@@ -2264,40 +2260,49 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
22642260

22652261
/*
22662262
* get_steps_using_prefix
2267-
*Generate list of PartitionPruneStepOp steps each consisting of given
2268-
*opstrategy
2269-
*
2270-
* To generate steps, step_lastexpr and step_lastcmpfn are appended to
2271-
* expressions and cmpfns, respectively, extracted from the clauses in
2272-
* 'prefix'. Actually, since 'prefix' may contain multiple clauses for the
2273-
* same partition key column, we must generate steps for various combinations
2274-
* of the clauses of different keys.
2275-
*
2276-
* For list/range partitioning, callers must ensure that step_nullkeys is
2277-
* NULL, and that prefix contains at least one clause for each of the
2278-
* partition keys earlier than one specified in step_lastkeyno if it's
2279-
* greater than zero. For hash partitioning, step_nullkeys is allowed to be
2280-
* non-NULL, but they must ensure that prefix contains at least one clause
2281-
* for each of the partition keys other than those specified in step_nullkeys
2282-
* and step_lastkeyno.
2283-
*
2284-
* For both cases, callers must also ensure that clauses in prefix are sorted
2285-
* in ascending order of their partition key numbers.
2263+
*Generate a list of PartitionPruneStepOps based on the given input.
2264+
*
2265+
* 'step_lastexpr' and 'step_lastcmpfn' are the Expr and comparison function
2266+
* belonging to the final partition key that we have a clause for. 'prefix'
2267+
* is a list of PartClauseInfos for partition key numbers prior to the given
2268+
* 'step_lastexpr' and 'step_lastcmpfn'. 'prefix' may contain multiple
2269+
* PartClauseInfos belonging to a single partition key. We will generate a
2270+
* PartitionPruneStepOp for each combination of the given PartClauseInfos
2271+
* using, at most, one PartClauseInfo per partition key.
2272+
*
2273+
* For LIST and RANGE partitioned tables, callers must ensure that
2274+
* step_nullkeys is NULL, and that prefix contains at least one clause for
2275+
* each of the partition keys prior to the key that 'step_lastexpr' and
2276+
* 'step_lastcmpfn'belong to.
2277+
*
2278+
* For HASH partitioned tables, callers must ensure that 'prefix' contains at
2279+
* least one clause for each of the partition keys apart from the final key
2280+
* (the expr and comparison function for the final key are in 'step_lastexpr'
2281+
* and 'step_lastcmpfn'). A bit set in step_nullkeys can substitute clauses
2282+
* in the 'prefix' list for any given key. If a bit is set in 'step_nullkeys'
2283+
* for a given key, then there must be no PartClauseInfo for that key in the
2284+
* 'prefix' list.
2285+
*
2286+
* For each of the above cases, callers must ensure that PartClauseInfos in
2287+
* 'prefix' are sorted in ascending order of keyno.
22862288
*/
22872289
staticList*
22882290
get_steps_using_prefix(GeneratePruningStepsContext*context,
22892291
StrategyNumberstep_opstrategy,
22902292
boolstep_op_is_ne,
22912293
Expr*step_lastexpr,
22922294
Oidstep_lastcmpfn,
2293-
intstep_lastkeyno,
22942295
Bitmapset*step_nullkeys,
22952296
List*prefix)
22962297
{
2298+
/* step_nullkeys must be empty for RANGE and LIST partitioned tables */
22972299
Assert(step_nullkeys==NULL||
22982300
context->rel->part_scheme->strategy==PARTITION_STRATEGY_HASH);
22992301

2300-
/* Quick exit if there are no values to prefix with. */
2302+
/*
2303+
* No recursive processing is required when 'prefix' is an empty list. This
2304+
* occurs when there is only 1 partition key column.
2305+
*/
23012306
if (list_length(prefix)==0)
23022307
{
23032308
PartitionPruneStep*step;
@@ -2311,26 +2316,31 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
23112316
returnlist_make1(step);
23122317
}
23132318

2314-
/* Recurse to generate steps forvarious combinations. */
2319+
/* Recurse to generate steps forevery combination of clauses. */
23152320
returnget_steps_using_prefix_recurse(context,
23162321
step_opstrategy,
23172322
step_op_is_ne,
23182323
step_lastexpr,
23192324
step_lastcmpfn,
2320-
step_lastkeyno,
23212325
step_nullkeys,
2326+
prefix,
23222327
list_head(prefix),
23232328
NIL,NIL);
23242329
}
23252330

23262331
/*
23272332
* get_steps_using_prefix_recurse
2328-
*Recursively generate combinations of clauses for different partition
2329-
*keys and start generating steps upon reaching clauses for the greatest
2330-
*column that is less than the one for which we're currently generating
2331-
*steps (that is, step_lastkeyno)
2333+
*Generate and return a list of PartitionPruneStepOps using the 'prefix'
2334+
*list of PartClauseInfos starting at the 'start' cell.
2335+
*
2336+
* When 'prefix' contains multiple PartClauseInfos for a single partition key
2337+
* we create a PartitionPruneStepOp for each combination of duplicated
2338+
* PartClauseInfos. The returned list will contain a PartitionPruneStepOp
2339+
* for each unique combination of input PartClauseInfos containing at most one
2340+
* PartClauseInfo per partition key.
23322341
*
2333-
* 'start' is where we should start iterating for the current invocation.
2342+
* 'prefix' is the input list of PartClauseInfos sorted by keyno.
2343+
* 'start' marks the cell that searching the 'prefix' list should start from.
23342344
* 'step_exprs' and 'step_cmpfns' each contains the expressions and cmpfns
23352345
* we've generated so far from the clauses for the previous part keys.
23362346
*/
@@ -2340,32 +2350,34 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
23402350
boolstep_op_is_ne,
23412351
Expr*step_lastexpr,
23422352
Oidstep_lastcmpfn,
2343-
intstep_lastkeyno,
23442353
Bitmapset*step_nullkeys,
2354+
List*prefix,
23452355
ListCell*start,
23462356
List*step_exprs,
23472357
List*step_cmpfns)
23482358
{
23492359
List*result=NIL;
23502360
ListCell*lc;
23512361
intcur_keyno;
2362+
intfinal_keyno;
23522363

23532364
/* Actually, recursion would be limited by PARTITION_MAX_KEYS. */
23542365
check_stack_depth();
23552366

2356-
/* Check if we need to recurse. */
23572367
Assert(start!=NULL);
23582368
cur_keyno= ((PartClauseInfo*)lfirst(start))->keyno;
2359-
if (cur_keyno<step_lastkeyno-1)
2369+
final_keyno= ((PartClauseInfo*)llast(prefix))->keyno;
2370+
2371+
/* Check if we need to recurse. */
2372+
if (cur_keyno<final_keyno)
23602373
{
23612374
PartClauseInfo*pc;
23622375
ListCell*next_start;
23632376

23642377
/*
2365-
* For each clause with cur_keyno, add its expr and cmpfn to
2366-
* step_exprs and step_cmpfns, respectively, and recurse after setting
2367-
* next_start to the ListCell of the first clause for the next
2368-
* partition key.
2378+
* Find the first PartClauseInfo belonging to the next partition key, the
2379+
* next recursive call must start iteration of the prefix list from that
2380+
* point.
23692381
*/
23702382
for_each_cell(lc,start)
23712383
{
@@ -2374,8 +2386,15 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
23742386
if (pc->keyno>cur_keyno)
23752387
break;
23762388
}
2389+
2390+
/* record where to start iterating in the next recursive call */
23772391
next_start=lc;
23782392

2393+
/*
2394+
* For each PartClauseInfo with keyno set to cur_keyno, add its expr and
2395+
* cmpfn to step_exprs and step_cmpfns, respectively, and recurse using
2396+
* 'next_start' as the starting point in the 'prefix' list.
2397+
*/
23792398
for_each_cell(lc,start)
23802399
{
23812400
List*moresteps;
@@ -2395,6 +2414,7 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
23952414
}
23962415
else
23972416
{
2417+
/* check the 'prefix' list is sorted correctly */
23982418
Assert(pc->keyno>cur_keyno);
23992419
break;
24002420
}
@@ -2404,8 +2424,8 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
24042424
step_op_is_ne,
24052425
step_lastexpr,
24062426
step_lastcmpfn,
2407-
step_lastkeyno,
24082427
step_nullkeys,
2428+
prefix,
24092429
next_start,
24102430
step_exprs1,
24112431
step_cmpfns1);
@@ -2422,8 +2442,8 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
24222442
* each clause with cur_keyno, which is all clauses from here onward
24232443
* till the end of the list. Note that for hash partitioning,
24242444
* step_nullkeys is allowed to be non-empty, in which case step_exprs
2425-
* would only contain expressions for theearlierpartition keys that
2426-
*are notspecified in step_nullkeys.
2445+
* would only contain expressions for the partition keys that are not
2446+
* specified in step_nullkeys.
24272447
*/
24282448
Assert(list_length(step_exprs)==cur_keyno||
24292449
!bms_is_empty(step_nullkeys));

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp