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

Commit45c4b2a

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 parente6b41d5 commit45c4b2a

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
@@ -165,15 +165,13 @@ static List *get_steps_using_prefix(GeneratePruningStepsContext *context,
165165
boolstep_op_is_ne,
166166
Expr*step_lastexpr,
167167
Oidstep_lastcmpfn,
168-
intstep_lastkeyno,
169168
Bitmapset*step_nullkeys,
170169
List*prefix);
171170
staticList*get_steps_using_prefix_recurse(GeneratePruningStepsContext*context,
172171
StrategyNumberstep_opstrategy,
173172
boolstep_op_is_ne,
174173
Expr*step_lastexpr,
175174
Oidstep_lastcmpfn,
176-
intstep_lastkeyno,
177175
Bitmapset*step_nullkeys,
178176
List*prefix,
179177
ListCell*start,
@@ -1411,7 +1409,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
14111409
pc->op_is_ne,
14121410
pc->expr,
14131411
pc->cmpfn,
1414-
0,
14151412
NULL,
14161413
NIL);
14171414
opsteps=list_concat(opsteps,pc_steps);
@@ -1537,7 +1534,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
15371534
pc->op_is_ne,
15381535
pc->expr,
15391536
pc->cmpfn,
1540-
pc->keyno,
15411537
NULL,
15421538
prefix);
15431539
opsteps=list_concat(opsteps,pc_steps);
@@ -1611,7 +1607,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
16111607
false,
16121608
pc->expr,
16131609
pc->cmpfn,
1614-
pc->keyno,
16151610
nullkeys,
16161611
prefix);
16171612
opsteps=list_concat(opsteps,pc_steps);
@@ -2251,40 +2246,49 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
22512246

22522247
/*
22532248
* get_steps_using_prefix
2254-
*Generate list of PartitionPruneStepOp steps each consisting of given
2255-
*opstrategy
2256-
*
2257-
* To generate steps, step_lastexpr and step_lastcmpfn are appended to
2258-
* expressions and cmpfns, respectively, extracted from the clauses in
2259-
* 'prefix'. Actually, since 'prefix' may contain multiple clauses for the
2260-
* same partition key column, we must generate steps for various combinations
2261-
* of the clauses of different keys.
2262-
*
2263-
* For list/range partitioning, callers must ensure that step_nullkeys is
2264-
* NULL, and that prefix contains at least one clause for each of the
2265-
* partition keys earlier than one specified in step_lastkeyno if it's
2266-
* greater than zero. For hash partitioning, step_nullkeys is allowed to be
2267-
* non-NULL, but they must ensure that prefix contains at least one clause
2268-
* for each of the partition keys other than those specified in step_nullkeys
2269-
* and step_lastkeyno.
2270-
*
2271-
* For both cases, callers must also ensure that clauses in prefix are sorted
2272-
* in ascending order of their partition key numbers.
2249+
*Generate a list of PartitionPruneStepOps based on the given input.
2250+
*
2251+
* 'step_lastexpr' and 'step_lastcmpfn' are the Expr and comparison function
2252+
* belonging to the final partition key that we have a clause for. 'prefix'
2253+
* is a list of PartClauseInfos for partition key numbers prior to the given
2254+
* 'step_lastexpr' and 'step_lastcmpfn'. 'prefix' may contain multiple
2255+
* PartClauseInfos belonging to a single partition key. We will generate a
2256+
* PartitionPruneStepOp for each combination of the given PartClauseInfos
2257+
* using, at most, one PartClauseInfo per partition key.
2258+
*
2259+
* For LIST and RANGE partitioned tables, callers must ensure that
2260+
* step_nullkeys is NULL, and that prefix contains at least one clause for
2261+
* each of the partition keys prior to the key that 'step_lastexpr' and
2262+
* 'step_lastcmpfn'belong to.
2263+
*
2264+
* For HASH partitioned tables, callers must ensure that 'prefix' contains at
2265+
* least one clause for each of the partition keys apart from the final key
2266+
* (the expr and comparison function for the final key are in 'step_lastexpr'
2267+
* and 'step_lastcmpfn'). A bit set in step_nullkeys can substitute clauses
2268+
* in the 'prefix' list for any given key. If a bit is set in 'step_nullkeys'
2269+
* for a given key, then there must be no PartClauseInfo for that key in the
2270+
* 'prefix' list.
2271+
*
2272+
* For each of the above cases, callers must ensure that PartClauseInfos in
2273+
* 'prefix' are sorted in ascending order of keyno.
22732274
*/
22742275
staticList*
22752276
get_steps_using_prefix(GeneratePruningStepsContext*context,
22762277
StrategyNumberstep_opstrategy,
22772278
boolstep_op_is_ne,
22782279
Expr*step_lastexpr,
22792280
Oidstep_lastcmpfn,
2280-
intstep_lastkeyno,
22812281
Bitmapset*step_nullkeys,
22822282
List*prefix)
22832283
{
2284+
/* step_nullkeys must be empty for RANGE and LIST partitioned tables */
22842285
Assert(step_nullkeys==NULL||
22852286
context->rel->part_scheme->strategy==PARTITION_STRATEGY_HASH);
22862287

2287-
/* Quick exit if there are no values to prefix with. */
2288+
/*
2289+
* No recursive processing is required when 'prefix' is an empty list. This
2290+
* occurs when there is only 1 partition key column.
2291+
*/
22882292
if (list_length(prefix)==0)
22892293
{
22902294
PartitionPruneStep*step;
@@ -2298,13 +2302,12 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
22982302
returnlist_make1(step);
22992303
}
23002304

2301-
/* Recurse to generate steps forvarious combinations. */
2305+
/* Recurse to generate steps forevery combination of clauses. */
23022306
returnget_steps_using_prefix_recurse(context,
23032307
step_opstrategy,
23042308
step_op_is_ne,
23052309
step_lastexpr,
23062310
step_lastcmpfn,
2307-
step_lastkeyno,
23082311
step_nullkeys,
23092312
prefix,
23102313
list_head(prefix),
@@ -2313,13 +2316,17 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
23132316

23142317
/*
23152318
* get_steps_using_prefix_recurse
2316-
*Recursively generate combinations of clauses for different partition
2317-
*keys and start generating steps upon reaching clauses for the greatest
2318-
*column that is less than the one for which we're currently generating
2319-
*steps (that is, step_lastkeyno)
2319+
*Generate and return a list of PartitionPruneStepOps using the 'prefix'
2320+
*list of PartClauseInfos starting at the 'start' cell.
2321+
*
2322+
* When 'prefix' contains multiple PartClauseInfos for a single partition key
2323+
* we create a PartitionPruneStepOp for each combination of duplicated
2324+
* PartClauseInfos. The returned list will contain a PartitionPruneStepOp
2325+
* for each unique combination of input PartClauseInfos containing at most one
2326+
* PartClauseInfo per partition key.
23202327
*
2321-
* 'prefix' is the list of PartClauseInfos.
2322-
* 'start'is where we should start iterating for the current invocation.
2328+
* 'prefix' is theinputlist of PartClauseInfos sorted by keyno.
2329+
* 'start'marks the cell that searching the 'prefix' list should start from.
23232330
* 'step_exprs' and 'step_cmpfns' each contains the expressions and cmpfns
23242331
* we've generated so far from the clauses for the previous part keys.
23252332
*/
@@ -2329,7 +2336,6 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
23292336
boolstep_op_is_ne,
23302337
Expr*step_lastexpr,
23312338
Oidstep_lastcmpfn,
2332-
intstep_lastkeyno,
23332339
Bitmapset*step_nullkeys,
23342340
List*prefix,
23352341
ListCell*start,
@@ -2339,23 +2345,25 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
23392345
List*result=NIL;
23402346
ListCell*lc;
23412347
intcur_keyno;
2348+
intfinal_keyno;
23422349

23432350
/* Actually, recursion would be limited by PARTITION_MAX_KEYS. */
23442351
check_stack_depth();
23452352

2346-
/* Check if we need to recurse. */
23472353
Assert(start!=NULL);
23482354
cur_keyno= ((PartClauseInfo*)lfirst(start))->keyno;
2349-
if (cur_keyno<step_lastkeyno-1)
2355+
final_keyno= ((PartClauseInfo*)llast(prefix))->keyno;
2356+
2357+
/* Check if we need to recurse. */
2358+
if (cur_keyno<final_keyno)
23502359
{
23512360
PartClauseInfo*pc;
23522361
ListCell*next_start;
23532362

23542363
/*
2355-
* For each clause with cur_keyno, add its expr and cmpfn to
2356-
* step_exprs and step_cmpfns, respectively, and recurse after setting
2357-
* next_start to the ListCell of the first clause for the next
2358-
* partition key.
2364+
* Find the first PartClauseInfo belonging to the next partition key, the
2365+
* next recursive call must start iteration of the prefix list from that
2366+
* point.
23592367
*/
23602368
for_each_cell(lc,prefix,start)
23612369
{
@@ -2364,8 +2372,15 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
23642372
if (pc->keyno>cur_keyno)
23652373
break;
23662374
}
2375+
2376+
/* record where to start iterating in the next recursive call */
23672377
next_start=lc;
23682378

2379+
/*
2380+
* For each PartClauseInfo with keyno set to cur_keyno, add its expr and
2381+
* cmpfn to step_exprs and step_cmpfns, respectively, and recurse using
2382+
* 'next_start' as the starting point in the 'prefix' list.
2383+
*/
23692384
for_each_cell(lc,prefix,start)
23702385
{
23712386
List*moresteps;
@@ -2385,6 +2400,7 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
23852400
}
23862401
else
23872402
{
2403+
/* check the 'prefix' list is sorted correctly */
23882404
Assert(pc->keyno>cur_keyno);
23892405
break;
23902406
}
@@ -2394,7 +2410,6 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
23942410
step_op_is_ne,
23952411
step_lastexpr,
23962412
step_lastcmpfn,
2397-
step_lastkeyno,
23982413
step_nullkeys,
23992414
prefix,
24002415
next_start,
@@ -2413,8 +2428,8 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
24132428
* each clause with cur_keyno, which is all clauses from here onward
24142429
* till the end of the list. Note that for hash partitioning,
24152430
* step_nullkeys is allowed to be non-empty, in which case step_exprs
2416-
* would only contain expressions for theearlierpartition keys that
2417-
*are notspecified in step_nullkeys.
2431+
* would only contain expressions for the partition keys that are not
2432+
* specified in step_nullkeys.
24182433
*/
24192434
Assert(list_length(step_exprs)==cur_keyno||
24202435
!bms_is_empty(step_nullkeys));

0 commit comments

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