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

Commite9f0311

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 parentd2761b6 commite9f0311

File tree

4 files changed

+218
-162
lines changed

4 files changed

+218
-162
lines changed

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

Lines changed: 17 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ static Cost bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel,
126126
List*paths);
127127
staticPathClauseUsage*classify_index_clause_usage(Path*path,
128128
List**clauselist);
129-
staticRelidsget_bitmap_tree_required_outer(Path*bitmapqual);
130129
staticvoidfind_indexpath_quals(Path*bitmapqual,List**quals,List**preds);
131130
staticintfind_list_position(Node*node,List**nodelist);
132131
staticboolcheck_index_only(RelOptInfo*rel,IndexOptInfo*index);
@@ -356,23 +355,16 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
356355
*/
357356
if (bitjoinpaths!=NIL)
358357
{
359-
List*path_outer;
360358
List*all_path_outers;
361359
ListCell*lc;
362360

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

374-
required_outer=get_bitmap_tree_required_outer(path);
375-
path_outer=lappend(path_outer,required_outer);
376368
if (!bms_equal_any(required_outer,all_path_outers))
377369
all_path_outers=lappend(all_path_outers,required_outer);
378370
}
@@ -387,16 +379,14 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
387379
doubleloop_count;
388380
BitmapHeapPath*bpath;
389381
ListCell*lcp;
390-
ListCell*lco;
391382

392383
/* Identify all the bitmap join paths needing no more than that */
393384
this_path_set=NIL;
394-
forboth(lcp,bitjoinpaths,lco,path_outer)
385+
foreach(lcp,bitjoinpaths)
395386
{
396387
Path*path= (Path*)lfirst(lcp);
397-
Relidsp_outers= (Relids)lfirst(lco);
398388

399-
if (bms_is_subset(p_outers,max_outers))
389+
if (bms_is_subset(PATH_REQ_OUTER(path),max_outers))
400390
this_path_set=lappend(this_path_set,path);
401391
}
402392

@@ -410,7 +400,7 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
410400
bitmapqual=choose_bitmap_and(root,rel,this_path_set);
411401

412402
/* And push that path into the mix */
413-
required_outer=get_bitmap_tree_required_outer(bitmapqual);
403+
required_outer=PATH_REQ_OUTER(bitmapqual);
414404
loop_count=get_loop_count(root,rel->relid,required_outer);
415405
bpath=create_bitmap_heap_path(root,rel,bitmapqual,
416406
required_outer,loop_count,0);
@@ -1611,25 +1601,19 @@ path_usage_comparator(const void *a, const void *b)
16111601

16121602
/*
16131603
* Estimate the cost of actually executing a bitmap scan with a single
1614-
* index path (no BitmapAnd, at least not at this level; but it could be
1615-
* a BitmapOr).
1604+
* index path (which could be a BitmapAnd or BitmapOr node).
16161605
*/
16171606
staticCost
16181607
bitmap_scan_cost_est(PlannerInfo*root,RelOptInfo*rel,Path*ipath)
16191608
{
16201609
BitmapHeapPathbpath;
1621-
Relidsrequired_outer;
1622-
1623-
/* Identify required outer rels, in case it's a parameterized scan */
1624-
required_outer=get_bitmap_tree_required_outer(ipath);
16251610

16261611
/* Set up a dummy BitmapHeapPath */
16271612
bpath.path.type=T_BitmapHeapPath;
16281613
bpath.path.pathtype=T_BitmapHeapScan;
16291614
bpath.path.parent=rel;
16301615
bpath.path.pathtarget=rel->reltarget;
1631-
bpath.path.param_info=get_baserel_parampathinfo(root,rel,
1632-
required_outer);
1616+
bpath.path.param_info=ipath->param_info;
16331617
bpath.path.pathkeys=NIL;
16341618
bpath.bitmapqual=ipath;
16351619

@@ -1638,10 +1622,13 @@ bitmap_scan_cost_est(PlannerInfo *root, RelOptInfo *rel, Path *ipath)
16381622
* Parallel bitmap heap path will be considered at later stage.
16391623
*/
16401624
bpath.path.parallel_workers=0;
1625+
1626+
/* Now we can do cost_bitmap_heap_scan */
16411627
cost_bitmap_heap_scan(&bpath.path,root,rel,
16421628
bpath.path.param_info,
16431629
ipath,
1644-
get_loop_count(root,rel->relid,required_outer));
1630+
get_loop_count(root,rel->relid,
1631+
PATH_REQ_OUTER(ipath)));
16451632

16461633
returnbpath.path.total_cost;
16471634
}
@@ -1653,46 +1640,15 @@ bitmap_scan_cost_est(PlannerInfo *root, RelOptInfo *rel, Path *ipath)
16531640
staticCost
16541641
bitmap_and_cost_est(PlannerInfo*root,RelOptInfo*rel,List*paths)
16551642
{
1656-
BitmapAndPathapath;
1657-
BitmapHeapPathbpath;
1658-
Relidsrequired_outer;
1659-
1660-
/* Set up a dummy BitmapAndPath */
1661-
apath.path.type=T_BitmapAndPath;
1662-
apath.path.pathtype=T_BitmapAnd;
1663-
apath.path.parent=rel;
1664-
apath.path.pathtarget=rel->reltarget;
1665-
apath.path.param_info=NULL;/* not used in bitmap trees */
1666-
apath.path.pathkeys=NIL;
1667-
apath.bitmapquals=paths;
1668-
cost_bitmap_and_node(&apath,root);
1669-
1670-
/* Identify required outer rels, in case it's a parameterized scan */
1671-
required_outer=get_bitmap_tree_required_outer((Path*)&apath);
1672-
1673-
/* Set up a dummy BitmapHeapPath */
1674-
bpath.path.type=T_BitmapHeapPath;
1675-
bpath.path.pathtype=T_BitmapHeapScan;
1676-
bpath.path.parent=rel;
1677-
bpath.path.pathtarget=rel->reltarget;
1678-
bpath.path.param_info=get_baserel_parampathinfo(root,rel,
1679-
required_outer);
1680-
bpath.path.pathkeys=NIL;
1681-
bpath.bitmapqual= (Path*)&apath;
1643+
BitmapAndPath*apath;
16821644

16831645
/*
1684-
*Check the cost of temporary path without considering parallelism.
1685-
*Parallel bitmap heap path will be considered at later stage.
1646+
*Might as well build a real BitmapAndPath here, as the work is slightly
1647+
*too complicated to be worth repeating just to save one palloc.
16861648
*/
1687-
bpath.path.parallel_workers=0;
1688-
1689-
/* Now we can do cost_bitmap_heap_scan */
1690-
cost_bitmap_heap_scan(&bpath.path,root,rel,
1691-
bpath.path.param_info,
1692-
(Path*)&apath,
1693-
get_loop_count(root,rel->relid,required_outer));
1649+
apath=create_bitmap_and_path(root,rel,paths);
16941650

1695-
returnbpath.path.total_cost;
1651+
returnbitmap_scan_cost_est(root,rel, (Path*)apath);
16961652
}
16971653

16981654

@@ -1763,49 +1719,6 @@ classify_index_clause_usage(Path *path, List **clauselist)
17631719
}
17641720

17651721

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

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

Lines changed: 46 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,11 +1118,27 @@ create_bitmap_and_path(PlannerInfo *root,
11181118
List*bitmapquals)
11191119
{
11201120
BitmapAndPath*pathnode=makeNode(BitmapAndPath);
1121+
Relidsrequired_outer=NULL;
1122+
ListCell*lc;
11211123

11221124
pathnode->path.pathtype=T_BitmapAnd;
11231125
pathnode->path.parent=rel;
11241126
pathnode->path.pathtarget=rel->reltarget;
1125-
pathnode->path.param_info=NULL;/* not used in bitmap trees */
1127+
1128+
/*
1129+
* Identify the required outer rels as the union of what the child paths
1130+
* depend on. (Alternatively, we could insist that the caller pass this
1131+
* in, but it's more convenient and reliable to compute it here.)
1132+
*/
1133+
foreach(lc,bitmapquals)
1134+
{
1135+
Path*bitmapqual= (Path*)lfirst(lc);
1136+
1137+
required_outer=bms_add_members(required_outer,
1138+
PATH_REQ_OUTER(bitmapqual));
1139+
}
1140+
pathnode->path.param_info=get_baserel_parampathinfo(root,rel,
1141+
required_outer);
11261142

11271143
/*
11281144
* Currently, a BitmapHeapPath, BitmapAndPath, or BitmapOrPath will be
@@ -1154,11 +1170,27 @@ create_bitmap_or_path(PlannerInfo *root,
11541170
List*bitmapquals)
11551171
{
11561172
BitmapOrPath*pathnode=makeNode(BitmapOrPath);
1173+
Relidsrequired_outer=NULL;
1174+
ListCell*lc;
11571175

11581176
pathnode->path.pathtype=T_BitmapOr;
11591177
pathnode->path.parent=rel;
11601178
pathnode->path.pathtarget=rel->reltarget;
1161-
pathnode->path.param_info=NULL;/* not used in bitmap trees */
1179+
1180+
/*
1181+
* Identify the required outer rels as the union of what the child paths
1182+
* depend on. (Alternatively, we could insist that the caller pass this
1183+
* in, but it's more convenient and reliable to compute it here.)
1184+
*/
1185+
foreach(lc,bitmapquals)
1186+
{
1187+
Path*bitmapqual= (Path*)lfirst(lc);
1188+
1189+
required_outer=bms_add_members(required_outer,
1190+
PATH_REQ_OUTER(bitmapqual));
1191+
}
1192+
pathnode->path.param_info=get_baserel_parampathinfo(root,rel,
1193+
required_outer);
11621194

11631195
/*
11641196
* Currently, a BitmapHeapPath, BitmapAndPath, or BitmapOrPath will be
@@ -3686,7 +3718,18 @@ do { \
36863718
!bms_overlap(PATH_REQ_OUTER(path),child_rel->top_parent_relids))
36873719
returnpath;
36883720

3689-
/* Reparameterize a copy of given path. */
3721+
/*
3722+
* If possible, reparameterize the given path, making a copy.
3723+
*
3724+
* This function is currently only applied to the inner side of a nestloop
3725+
* join that is being partitioned by the partitionwise-join code. Hence,
3726+
* we need only support path types that plausibly arise in that context.
3727+
* (In particular, supporting sorted path types would be a waste of code
3728+
* and cycles: even if we translated them here, they'd just lose in
3729+
* subsequent cost comparisons.) If we do see an unsupported path type,
3730+
* that just means we won't be able to generate a partitionwise-join plan
3731+
* using that path type.
3732+
*/
36903733
switch (nodeTag(path))
36913734
{
36923735
caseT_Path:
@@ -3734,20 +3777,6 @@ do { \
37343777
}
37353778
break;
37363779

3737-
caseT_TidPath:
3738-
{
3739-
TidPath*tpath;
3740-
3741-
/*
3742-
* TidPath contains tidquals, which do not contain any
3743-
* external parameters per create_tidscan_path(). So don't
3744-
* bother to translate those.
3745-
*/
3746-
FLAT_COPY_PATH(tpath,path,TidPath);
3747-
new_path= (Path*)tpath;
3748-
}
3749-
break;
3750-
37513780
caseT_ForeignPath:
37523781
{
37533782
ForeignPath*fpath;
@@ -3838,37 +3867,6 @@ do { \
38383867
}
38393868
break;
38403869

3841-
caseT_MergeAppendPath:
3842-
{
3843-
MergeAppendPath*mapath;
3844-
3845-
FLAT_COPY_PATH(mapath,path,MergeAppendPath);
3846-
REPARAMETERIZE_CHILD_PATH_LIST(mapath->subpaths);
3847-
new_path= (Path*)mapath;
3848-
}
3849-
break;
3850-
3851-
caseT_MaterialPath:
3852-
{
3853-
MaterialPath*mpath;
3854-
3855-
FLAT_COPY_PATH(mpath,path,MaterialPath);
3856-
REPARAMETERIZE_CHILD_PATH(mpath->subpath);
3857-
new_path= (Path*)mpath;
3858-
}
3859-
break;
3860-
3861-
caseT_UniquePath:
3862-
{
3863-
UniquePath*upath;
3864-
3865-
FLAT_COPY_PATH(upath,path,UniquePath);
3866-
REPARAMETERIZE_CHILD_PATH(upath->subpath);
3867-
ADJUST_CHILD_ATTRS(upath->uniq_exprs);
3868-
new_path= (Path*)upath;
3869-
}
3870-
break;
3871-
38723870
caseT_GatherPath:
38733871
{
38743872
GatherPath*gpath;
@@ -3879,16 +3877,6 @@ do { \
38793877
}
38803878
break;
38813879

3882-
caseT_GatherMergePath:
3883-
{
3884-
GatherMergePath*gmpath;
3885-
3886-
FLAT_COPY_PATH(gmpath,path,GatherMergePath);
3887-
REPARAMETERIZE_CHILD_PATH(gmpath->subpath);
3888-
new_path= (Path*)gmpath;
3889-
}
3890-
break;
3891-
38923880
default:
38933881

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

0 commit comments

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