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

Commit916adc7

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 parent5143f76 commit916adc7

File tree

3 files changed

+315
-62
lines changed

3 files changed

+315
-62
lines changed

‎src/backend/partitioning/partprune.c‎

Lines changed: 59 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,13 @@ static List *get_steps_using_prefix(GeneratePruningStepsContext *context,
167167
boolstep_op_is_ne,
168168
Expr*step_lastexpr,
169169
Oidstep_lastcmpfn,
170-
intstep_lastkeyno,
171170
Bitmapset*step_nullkeys,
172171
List*prefix);
173172
staticList*get_steps_using_prefix_recurse(GeneratePruningStepsContext*context,
174173
StrategyNumberstep_opstrategy,
175174
boolstep_op_is_ne,
176175
Expr*step_lastexpr,
177176
Oidstep_lastcmpfn,
178-
intstep_lastkeyno,
179177
Bitmapset*step_nullkeys,
180178
List*prefix,
181179
ListCell*start,
@@ -1531,7 +1529,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
15311529
pc->op_is_ne,
15321530
pc->expr,
15331531
pc->cmpfn,
1534-
0,
15351532
NULL,
15361533
NIL);
15371534
opsteps=list_concat(opsteps,pc_steps);
@@ -1657,7 +1654,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
16571654
pc->op_is_ne,
16581655
pc->expr,
16591656
pc->cmpfn,
1660-
pc->keyno,
16611657
NULL,
16621658
prefix);
16631659
opsteps=list_concat(opsteps,pc_steps);
@@ -1731,7 +1727,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
17311727
false,
17321728
pc->expr,
17331729
pc->cmpfn,
1734-
pc->keyno,
17351730
nullkeys,
17361731
prefix);
17371732
opsteps=list_concat(opsteps,pc_steps);
@@ -2351,40 +2346,49 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
23512346

23522347
/*
23532348
* get_steps_using_prefix
2354-
*Generate list of PartitionPruneStepOp steps each consisting of given
2355-
*opstrategy
2356-
*
2357-
* To generate steps, step_lastexpr and step_lastcmpfn are appended to
2358-
* expressions and cmpfns, respectively, extracted from the clauses in
2359-
* 'prefix'. Actually, since 'prefix' may contain multiple clauses for the
2360-
* same partition key column, we must generate steps for various combinations
2361-
* of the clauses of different keys.
2362-
*
2363-
* For list/range partitioning, callers must ensure that step_nullkeys is
2364-
* NULL, and that prefix contains at least one clause for each of the
2365-
* partition keys earlier than one specified in step_lastkeyno if it's
2366-
* greater than zero. For hash partitioning, step_nullkeys is allowed to be
2367-
* non-NULL, but they must ensure that prefix contains at least one clause
2368-
* for each of the partition keys other than those specified in step_nullkeys
2369-
* and step_lastkeyno.
2370-
*
2371-
* For both cases, callers must also ensure that clauses in prefix are sorted
2372-
* in ascending order of their partition key numbers.
2349+
*Generate a list of PartitionPruneStepOps based on the given input.
2350+
*
2351+
* 'step_lastexpr' and 'step_lastcmpfn' are the Expr and comparison function
2352+
* belonging to the final partition key that we have a clause for. 'prefix'
2353+
* is a list of PartClauseInfos for partition key numbers prior to the given
2354+
* 'step_lastexpr' and 'step_lastcmpfn'. 'prefix' may contain multiple
2355+
* PartClauseInfos belonging to a single partition key. We will generate a
2356+
* PartitionPruneStepOp for each combination of the given PartClauseInfos
2357+
* using, at most, one PartClauseInfo per partition key.
2358+
*
2359+
* For LIST and RANGE partitioned tables, callers must ensure that
2360+
* step_nullkeys is NULL, and that prefix contains at least one clause for
2361+
* each of the partition keys prior to the key that 'step_lastexpr' and
2362+
* 'step_lastcmpfn'belong to.
2363+
*
2364+
* For HASH partitioned tables, callers must ensure that 'prefix' contains at
2365+
* least one clause for each of the partition keys apart from the final key
2366+
* (the expr and comparison function for the final key are in 'step_lastexpr'
2367+
* and 'step_lastcmpfn'). A bit set in step_nullkeys can substitute clauses
2368+
* in the 'prefix' list for any given key. If a bit is set in 'step_nullkeys'
2369+
* for a given key, then there must be no PartClauseInfo for that key in the
2370+
* 'prefix' list.
2371+
*
2372+
* For each of the above cases, callers must ensure that PartClauseInfos in
2373+
* 'prefix' are sorted in ascending order of keyno.
23732374
*/
23742375
staticList*
23752376
get_steps_using_prefix(GeneratePruningStepsContext*context,
23762377
StrategyNumberstep_opstrategy,
23772378
boolstep_op_is_ne,
23782379
Expr*step_lastexpr,
23792380
Oidstep_lastcmpfn,
2380-
intstep_lastkeyno,
23812381
Bitmapset*step_nullkeys,
23822382
List*prefix)
23832383
{
2384+
/* step_nullkeys must be empty for RANGE and LIST partitioned tables */
23842385
Assert(step_nullkeys==NULL||
23852386
context->rel->part_scheme->strategy==PARTITION_STRATEGY_HASH);
23862387

2387-
/* Quick exit if there are no values to prefix with. */
2388+
/*
2389+
* No recursive processing is required when 'prefix' is an empty list. This
2390+
* occurs when there is only 1 partition key column.
2391+
*/
23882392
if (list_length(prefix)==0)
23892393
{
23902394
PartitionPruneStep*step;
@@ -2398,13 +2402,12 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
23982402
returnlist_make1(step);
23992403
}
24002404

2401-
/* Recurse to generate steps forvarious combinations. */
2405+
/* Recurse to generate steps forevery combination of clauses. */
24022406
returnget_steps_using_prefix_recurse(context,
24032407
step_opstrategy,
24042408
step_op_is_ne,
24052409
step_lastexpr,
24062410
step_lastcmpfn,
2407-
step_lastkeyno,
24082411
step_nullkeys,
24092412
prefix,
24102413
list_head(prefix),
@@ -2413,13 +2416,17 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
24132416

24142417
/*
24152418
* get_steps_using_prefix_recurse
2416-
*Recursively generate combinations of clauses for different partition
2417-
*keys and start generating steps upon reaching clauses for the greatest
2418-
*column that is less than the one for which we're currently generating
2419-
*steps (that is, step_lastkeyno)
2419+
*Generate and return a list of PartitionPruneStepOps using the 'prefix'
2420+
*list of PartClauseInfos starting at the 'start' cell.
2421+
*
2422+
* When 'prefix' contains multiple PartClauseInfos for a single partition key
2423+
* we create a PartitionPruneStepOp for each combination of duplicated
2424+
* PartClauseInfos. The returned list will contain a PartitionPruneStepOp
2425+
* for each unique combination of input PartClauseInfos containing at most one
2426+
* PartClauseInfo per partition key.
24202427
*
2421-
* 'prefix' is the list of PartClauseInfos.
2422-
* 'start'is where we should start iterating for the current invocation.
2428+
* 'prefix' is theinputlist of PartClauseInfos sorted by keyno.
2429+
* 'start'marks the cell that searching the 'prefix' list should start from.
24232430
* 'step_exprs' and 'step_cmpfns' each contains the expressions and cmpfns
24242431
* we've generated so far from the clauses for the previous part keys.
24252432
*/
@@ -2429,7 +2436,6 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
24292436
boolstep_op_is_ne,
24302437
Expr*step_lastexpr,
24312438
Oidstep_lastcmpfn,
2432-
intstep_lastkeyno,
24332439
Bitmapset*step_nullkeys,
24342440
List*prefix,
24352441
ListCell*start,
@@ -2439,23 +2445,25 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
24392445
List*result=NIL;
24402446
ListCell*lc;
24412447
intcur_keyno;
2448+
intfinal_keyno;
24422449

24432450
/* Actually, recursion would be limited by PARTITION_MAX_KEYS. */
24442451
check_stack_depth();
24452452

2446-
/* Check if we need to recurse. */
24472453
Assert(start!=NULL);
24482454
cur_keyno= ((PartClauseInfo*)lfirst(start))->keyno;
2449-
if (cur_keyno<step_lastkeyno-1)
2455+
final_keyno= ((PartClauseInfo*)llast(prefix))->keyno;
2456+
2457+
/* Check if we need to recurse. */
2458+
if (cur_keyno<final_keyno)
24502459
{
24512460
PartClauseInfo*pc;
24522461
ListCell*next_start;
24532462

24542463
/*
2455-
* For each clause with cur_keyno, add its expr and cmpfn to
2456-
* step_exprs and step_cmpfns, respectively, and recurse after setting
2457-
* next_start to the ListCell of the first clause for the next
2458-
* partition key.
2464+
* Find the first PartClauseInfo belonging to the next partition key, the
2465+
* next recursive call must start iteration of the prefix list from that
2466+
* point.
24592467
*/
24602468
for_each_cell(lc,prefix,start)
24612469
{
@@ -2464,8 +2472,15 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
24642472
if (pc->keyno>cur_keyno)
24652473
break;
24662474
}
2475+
2476+
/* record where to start iterating in the next recursive call */
24672477
next_start=lc;
24682478

2479+
/*
2480+
* For each PartClauseInfo with keyno set to cur_keyno, add its expr and
2481+
* cmpfn to step_exprs and step_cmpfns, respectively, and recurse using
2482+
* 'next_start' as the starting point in the 'prefix' list.
2483+
*/
24692484
for_each_cell(lc,prefix,start)
24702485
{
24712486
List*moresteps;
@@ -2485,6 +2500,7 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
24852500
}
24862501
else
24872502
{
2503+
/* check the 'prefix' list is sorted correctly */
24882504
Assert(pc->keyno>cur_keyno);
24892505
break;
24902506
}
@@ -2494,7 +2510,6 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
24942510
step_op_is_ne,
24952511
step_lastexpr,
24962512
step_lastcmpfn,
2497-
step_lastkeyno,
24982513
step_nullkeys,
24992514
prefix,
25002515
next_start,
@@ -2513,8 +2528,8 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
25132528
* each clause with cur_keyno, which is all clauses from here onward
25142529
* till the end of the list. Note that for hash partitioning,
25152530
* step_nullkeys is allowed to be non-empty, in which case step_exprs
2516-
* would only contain expressions for theearlierpartition keys that
2517-
*are notspecified in step_nullkeys.
2531+
* would only contain expressions for the partition keys that are not
2532+
* specified in step_nullkeys.
25182533
*/
25192534
Assert(list_length(step_exprs)==cur_keyno||
25202535
!bms_is_empty(step_nullkeys));

0 commit comments

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