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

Commit689696c

Browse files
committed
Fix bitmap AND/OR scans on the inside of a nestloop partition-wise join.
reparameterize_path_by_child() failed to reparameterize BitmapAndand BitmapOr paths. This matters only if such a path is chosen asthe inside of a nestloop partition-wise join, where we have to passin parameters from the outside of the nestloop. If that did happen,we generated a bad plan that would likely lead to crashes at execution.This is not entirely reparameterize_path_by_child()'s fault though;it's the victim of an ancient decision (my ancient decision, I think)to not bother filling in param_info in BitmapAnd/Or path nodes. Thatcaused the function to believe that such nodes and their childrencontain no parameter references and so need not be processed.In hindsight that decision looks pretty penny-wise and pound-foolish:while it saves a few cycles during path node setup, we do commonlyneed the information later. In particular, by reversing the decisionand requiring valid param_info data in all nodes of a bitmap pathtree, we can get rid of indxpath.c's get_bitmap_tree_required_outer()function, which computed the data on-demand. It's not unlikely thatthat nets out as a savings of cycles in many scenarios. A coupleof other things in indxpath.c can be simplified as well.While here, get rid of some cases in reparameterize_path_by_child()that are visibly dead or useless, given that we only care aboutreparameterizing paths that can be on the inside of a parameterizednestloop. This case reminds one of the maxim that untested codeprobably does not work, so I'm unwilling to leave unreachable codein this function. (I did leave the T_Gather case in place eventhough it's not reached in the regression tests. It's not veryclear to me when the planner might prefer to put Gather belowrather than above a nestloop, but at least in principle the casemight be interesting.)Per bug #16536, originally from Arne Roland but with a test caseby Andrew Gierth. Back-patch to v11 where this code came in.Discussion:https://postgr.es/m/16536-2213ee0b3aad41fd@postgresql.org
1 parentde8feb1 commit689696c

File tree

4 files changed

+211
-158
lines changed

4 files changed

+211
-158
lines changed

‎src/backend/optimizer/path/indxpath.c‎

Lines changed: 17 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ static Cost bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel,
122122
List*paths);
123123
staticPathClauseUsage*classify_index_clause_usage(Path*path,
124124
List**clauselist);
125-
staticRelidsget_bitmap_tree_required_outer(Path*bitmapqual);
126125
staticvoidfind_indexpath_quals(Path*bitmapqual,List**quals,List**preds);
127126
staticintfind_list_position(Node*node,List**nodelist);
128127
staticboolcheck_index_only(RelOptInfo*rel,IndexOptInfo*index);
@@ -357,23 +356,16 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
357356
*/
358357
if (bitjoinpaths!=NIL)
359358
{
360-
List*path_outer;
361359
List*all_path_outers;
362360
ListCell*lc;
363361

364-
/*
365-
* path_outer holds the parameterization of each path in bitjoinpaths
366-
* (to save recalculating that several times), while all_path_outers
367-
* holds all distinct parameterization sets.
368-
*/
369-
path_outer=all_path_outers=NIL;
362+
/* Identify each distinct parameterization seen in bitjoinpaths */
363+
all_path_outers=NIL;
370364
foreach(lc,bitjoinpaths)
371365
{
372366
Path*path= (Path*)lfirst(lc);
373-
Relidsrequired_outer;
367+
Relidsrequired_outer=PATH_REQ_OUTER(path);
374368

375-
required_outer=get_bitmap_tree_required_outer(path);
376-
path_outer=lappend(path_outer,required_outer);
377369
if (!bms_equal_any(required_outer,all_path_outers))
378370
all_path_outers=lappend(all_path_outers,required_outer);
379371
}
@@ -388,16 +380,14 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
388380
doubleloop_count;
389381
BitmapHeapPath*bpath;
390382
ListCell*lcp;
391-
ListCell*lco;
392383

393384
/* Identify all the bitmap join paths needing no more than that */
394385
this_path_set=NIL;
395-
forboth(lcp,bitjoinpaths,lco,path_outer)
386+
foreach(lcp,bitjoinpaths)
396387
{
397388
Path*path= (Path*)lfirst(lcp);
398-
Relidsp_outers= (Relids)lfirst(lco);
399389

400-
if (bms_is_subset(p_outers,max_outers))
390+
if (bms_is_subset(PATH_REQ_OUTER(path),max_outers))
401391
this_path_set=lappend(this_path_set,path);
402392
}
403393

@@ -411,7 +401,7 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
411401
bitmapqual=choose_bitmap_and(root,rel,this_path_set);
412402

413403
/* And push that path into the mix */
414-
required_outer=get_bitmap_tree_required_outer(bitmapqual);
404+
required_outer=PATH_REQ_OUTER(bitmapqual);
415405
loop_count=get_loop_count(root,rel->relid,required_outer);
416406
bpath=create_bitmap_heap_path(root,rel,bitmapqual,
417407
required_outer,loop_count,0);
@@ -1601,25 +1591,19 @@ path_usage_comparator(const void *a, const void *b)
16011591

16021592
/*
16031593
* Estimate the cost of actually executing a bitmap scan with a single
1604-
* index path (no BitmapAnd, at least not at this level; but it could be
1605-
* a BitmapOr).
1594+
* index path (which could be a BitmapAnd or BitmapOr node).
16061595
*/
16071596
staticCost
16081597
bitmap_scan_cost_est(PlannerInfo*root,RelOptInfo*rel,Path*ipath)
16091598
{
16101599
BitmapHeapPathbpath;
1611-
Relidsrequired_outer;
1612-
1613-
/* Identify required outer rels, in case it's a parameterized scan */
1614-
required_outer=get_bitmap_tree_required_outer(ipath);
16151600

16161601
/* Set up a dummy BitmapHeapPath */
16171602
bpath.path.type=T_BitmapHeapPath;
16181603
bpath.path.pathtype=T_BitmapHeapScan;
16191604
bpath.path.parent=rel;
16201605
bpath.path.pathtarget=rel->reltarget;
1621-
bpath.path.param_info=get_baserel_parampathinfo(root,rel,
1622-
required_outer);
1606+
bpath.path.param_info=ipath->param_info;
16231607
bpath.path.pathkeys=NIL;
16241608
bpath.bitmapqual=ipath;
16251609

@@ -1628,10 +1612,13 @@ bitmap_scan_cost_est(PlannerInfo *root, RelOptInfo *rel, Path *ipath)
16281612
* Parallel bitmap heap path will be considered at later stage.
16291613
*/
16301614
bpath.path.parallel_workers=0;
1615+
1616+
/* Now we can do cost_bitmap_heap_scan */
16311617
cost_bitmap_heap_scan(&bpath.path,root,rel,
16321618
bpath.path.param_info,
16331619
ipath,
1634-
get_loop_count(root,rel->relid,required_outer));
1620+
get_loop_count(root,rel->relid,
1621+
PATH_REQ_OUTER(ipath)));
16351622

16361623
returnbpath.path.total_cost;
16371624
}
@@ -1643,46 +1630,15 @@ bitmap_scan_cost_est(PlannerInfo *root, RelOptInfo *rel, Path *ipath)
16431630
staticCost
16441631
bitmap_and_cost_est(PlannerInfo*root,RelOptInfo*rel,List*paths)
16451632
{
1646-
BitmapAndPathapath;
1647-
BitmapHeapPathbpath;
1648-
Relidsrequired_outer;
1649-
1650-
/* Set up a dummy BitmapAndPath */
1651-
apath.path.type=T_BitmapAndPath;
1652-
apath.path.pathtype=T_BitmapAnd;
1653-
apath.path.parent=rel;
1654-
apath.path.pathtarget=rel->reltarget;
1655-
apath.path.param_info=NULL;/* not used in bitmap trees */
1656-
apath.path.pathkeys=NIL;
1657-
apath.bitmapquals=paths;
1658-
cost_bitmap_and_node(&apath,root);
1659-
1660-
/* Identify required outer rels, in case it's a parameterized scan */
1661-
required_outer=get_bitmap_tree_required_outer((Path*)&apath);
1662-
1663-
/* Set up a dummy BitmapHeapPath */
1664-
bpath.path.type=T_BitmapHeapPath;
1665-
bpath.path.pathtype=T_BitmapHeapScan;
1666-
bpath.path.parent=rel;
1667-
bpath.path.pathtarget=rel->reltarget;
1668-
bpath.path.param_info=get_baserel_parampathinfo(root,rel,
1669-
required_outer);
1670-
bpath.path.pathkeys=NIL;
1671-
bpath.bitmapqual= (Path*)&apath;
1633+
BitmapAndPath*apath;
16721634

16731635
/*
1674-
*Check the cost of temporary path without considering parallelism.
1675-
*Parallel bitmap heap path will be considered at later stage.
1636+
*Might as well build a real BitmapAndPath here, as the work is slightly
1637+
*too complicated to be worth repeating just to save one palloc.
16761638
*/
1677-
bpath.path.parallel_workers=0;
1678-
1679-
/* Now we can do cost_bitmap_heap_scan */
1680-
cost_bitmap_heap_scan(&bpath.path,root,rel,
1681-
bpath.path.param_info,
1682-
(Path*)&apath,
1683-
get_loop_count(root,rel->relid,required_outer));
1639+
apath=create_bitmap_and_path(root,rel,paths);
16841640

1685-
returnbpath.path.total_cost;
1641+
returnbitmap_scan_cost_est(root,rel, (Path*)apath);
16861642
}
16871643

16881644

@@ -1753,49 +1709,6 @@ classify_index_clause_usage(Path *path, List **clauselist)
17531709
}
17541710

17551711

1756-
/*
1757-
* get_bitmap_tree_required_outer
1758-
*Find the required outer rels for a bitmap tree (index/and/or)
1759-
*
1760-
* We don't associate any particular parameterization with a BitmapAnd or
1761-
* BitmapOr node; however, the IndexPaths have parameterization info, in
1762-
* their capacity as standalone access paths. The parameterization required
1763-
* for the bitmap heap scan node is the union of rels referenced in the
1764-
* child IndexPaths.
1765-
*/
1766-
staticRelids
1767-
get_bitmap_tree_required_outer(Path*bitmapqual)
1768-
{
1769-
Relidsresult=NULL;
1770-
ListCell*lc;
1771-
1772-
if (IsA(bitmapqual,IndexPath))
1773-
{
1774-
returnbms_copy(PATH_REQ_OUTER(bitmapqual));
1775-
}
1776-
elseif (IsA(bitmapqual,BitmapAndPath))
1777-
{
1778-
foreach(lc, ((BitmapAndPath*)bitmapqual)->bitmapquals)
1779-
{
1780-
result=bms_join(result,
1781-
get_bitmap_tree_required_outer((Path*)lfirst(lc)));
1782-
}
1783-
}
1784-
elseif (IsA(bitmapqual,BitmapOrPath))
1785-
{
1786-
foreach(lc, ((BitmapOrPath*)bitmapqual)->bitmapquals)
1787-
{
1788-
result=bms_join(result,
1789-
get_bitmap_tree_required_outer((Path*)lfirst(lc)));
1790-
}
1791-
}
1792-
else
1793-
elog(ERROR,"unrecognized node type: %d",nodeTag(bitmapqual));
1794-
1795-
returnresult;
1796-
}
1797-
1798-
17991712
/*
18001713
* find_indexpath_quals
18011714
*

‎src/backend/optimizer/util/pathnode.c‎

Lines changed: 46 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,11 +1081,27 @@ create_bitmap_and_path(PlannerInfo *root,
10811081
List*bitmapquals)
10821082
{
10831083
BitmapAndPath*pathnode=makeNode(BitmapAndPath);
1084+
Relidsrequired_outer=NULL;
1085+
ListCell*lc;
10841086

10851087
pathnode->path.pathtype=T_BitmapAnd;
10861088
pathnode->path.parent=rel;
10871089
pathnode->path.pathtarget=rel->reltarget;
1088-
pathnode->path.param_info=NULL;/* not used in bitmap trees */
1090+
1091+
/*
1092+
* Identify the required outer rels as the union of what the child paths
1093+
* depend on. (Alternatively, we could insist that the caller pass this
1094+
* in, but it's more convenient and reliable to compute it here.)
1095+
*/
1096+
foreach(lc,bitmapquals)
1097+
{
1098+
Path*bitmapqual= (Path*)lfirst(lc);
1099+
1100+
required_outer=bms_add_members(required_outer,
1101+
PATH_REQ_OUTER(bitmapqual));
1102+
}
1103+
pathnode->path.param_info=get_baserel_parampathinfo(root,rel,
1104+
required_outer);
10891105

10901106
/*
10911107
* Currently, a BitmapHeapPath, BitmapAndPath, or BitmapOrPath will be
@@ -1117,11 +1133,27 @@ create_bitmap_or_path(PlannerInfo *root,
11171133
List*bitmapquals)
11181134
{
11191135
BitmapOrPath*pathnode=makeNode(BitmapOrPath);
1136+
Relidsrequired_outer=NULL;
1137+
ListCell*lc;
11201138

11211139
pathnode->path.pathtype=T_BitmapOr;
11221140
pathnode->path.parent=rel;
11231141
pathnode->path.pathtarget=rel->reltarget;
1124-
pathnode->path.param_info=NULL;/* not used in bitmap trees */
1142+
1143+
/*
1144+
* Identify the required outer rels as the union of what the child paths
1145+
* depend on. (Alternatively, we could insist that the caller pass this
1146+
* in, but it's more convenient and reliable to compute it here.)
1147+
*/
1148+
foreach(lc,bitmapquals)
1149+
{
1150+
Path*bitmapqual= (Path*)lfirst(lc);
1151+
1152+
required_outer=bms_add_members(required_outer,
1153+
PATH_REQ_OUTER(bitmapqual));
1154+
}
1155+
pathnode->path.param_info=get_baserel_parampathinfo(root,rel,
1156+
required_outer);
11251157

11261158
/*
11271159
* Currently, a BitmapHeapPath, BitmapAndPath, or BitmapOrPath will be
@@ -3885,7 +3917,18 @@ do { \
38853917
!bms_overlap(PATH_REQ_OUTER(path),child_rel->top_parent_relids))
38863918
returnpath;
38873919

3888-
/* Reparameterize a copy of given path. */
3920+
/*
3921+
* If possible, reparameterize the given path, making a copy.
3922+
*
3923+
* This function is currently only applied to the inner side of a nestloop
3924+
* join that is being partitioned by the partitionwise-join code. Hence,
3925+
* we need only support path types that plausibly arise in that context.
3926+
* (In particular, supporting sorted path types would be a waste of code
3927+
* and cycles: even if we translated them here, they'd just lose in
3928+
* subsequent cost comparisons.) If we do see an unsupported path type,
3929+
* that just means we won't be able to generate a partitionwise-join plan
3930+
* using that path type.
3931+
*/
38893932
switch (nodeTag(path))
38903933
{
38913934
caseT_Path:
@@ -3932,16 +3975,6 @@ do { \
39323975
}
39333976
break;
39343977

3935-
caseT_TidPath:
3936-
{
3937-
TidPath*tpath;
3938-
3939-
FLAT_COPY_PATH(tpath,path,TidPath);
3940-
ADJUST_CHILD_ATTRS(tpath->tidquals);
3941-
new_path= (Path*)tpath;
3942-
}
3943-
break;
3944-
39453978
caseT_ForeignPath:
39463979
{
39473980
ForeignPath*fpath;
@@ -4032,37 +4065,6 @@ do { \
40324065
}
40334066
break;
40344067

4035-
caseT_MergeAppendPath:
4036-
{
4037-
MergeAppendPath*mapath;
4038-
4039-
FLAT_COPY_PATH(mapath,path,MergeAppendPath);
4040-
REPARAMETERIZE_CHILD_PATH_LIST(mapath->subpaths);
4041-
new_path= (Path*)mapath;
4042-
}
4043-
break;
4044-
4045-
caseT_MaterialPath:
4046-
{
4047-
MaterialPath*mpath;
4048-
4049-
FLAT_COPY_PATH(mpath,path,MaterialPath);
4050-
REPARAMETERIZE_CHILD_PATH(mpath->subpath);
4051-
new_path= (Path*)mpath;
4052-
}
4053-
break;
4054-
4055-
caseT_UniquePath:
4056-
{
4057-
UniquePath*upath;
4058-
4059-
FLAT_COPY_PATH(upath,path,UniquePath);
4060-
REPARAMETERIZE_CHILD_PATH(upath->subpath);
4061-
ADJUST_CHILD_ATTRS(upath->uniq_exprs);
4062-
new_path= (Path*)upath;
4063-
}
4064-
break;
4065-
40664068
caseT_GatherPath:
40674069
{
40684070
GatherPath*gpath;
@@ -4073,16 +4075,6 @@ do { \
40734075
}
40744076
break;
40754077

4076-
caseT_GatherMergePath:
4077-
{
4078-
GatherMergePath*gmpath;
4079-
4080-
FLAT_COPY_PATH(gmpath,path,GatherMergePath);
4081-
REPARAMETERIZE_CHILD_PATH(gmpath->subpath);
4082-
new_path= (Path*)gmpath;
4083-
}
4084-
break;
4085-
40864078
default:
40874079

40884080
/* We don't know how to reparameterize this path. */

0 commit comments

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