Skip to content

Commit ff38837

Browse files
tglsfdctglsfdc
committed
Fix nasty bug in optimization of multiway joins: optimizer
would sometimes generate a plan that omitted a sort step before merge.
1 parent 97c52ab commit ff38837

File tree

4 files changed

+35
-41
lines changed

4 files changed

+35
-41
lines changed

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

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/hashutils.c,v 1.14 1999/02/22 05:26:18 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/hashutils.c,v 1.15 1999/04/03 00:18:27 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -29,19 +29,20 @@ static HashInfo *match_hashop_hashinfo(Oid hashop, List *hashinfo_list);
2929
* hash operator.
3030
*
3131
* 'restrictinfo_list' is the list of restrictinfo nodes
32-
* 'inner_relid' is the relid of the inner join relation
32+
* 'inner_relids' is the list of relids in the inner join relation
33+
* (used to determine whether a join var is inner or outer)
3334
*
3435
* Returns the new list of hashinfo nodes.
3536
*
3637
*/
3738
List *
3839
group_clauses_by_hashop(List *restrictinfo_list,
39-
int inner_relid)
40+
Relids inner_relids)
4041
{
4142
List *hashinfo_list = NIL;
42-
RestrictInfo *restrictinfo = (RestrictInfo *) NULL;
43-
List *i = NIL;
44-
Oid hashjoinop = 0;
43+
RestrictInfo *restrictinfo;
44+
List *i;
45+
Oid hashjoinop;
4546

4647
foreach(i, restrictinfo_list)
4748
{
@@ -54,23 +55,21 @@ group_clauses_by_hashop(List *restrictinfo_list,
5455
*/
5556
if (hashjoinop)
5657
{
57-
HashInfo *xhashinfo = (HashInfo *) NULL;
5858
Expr *clause = restrictinfo->clause;
5959
Var *leftop = get_leftop(clause);
6060
Var *rightop = get_rightop(clause);
61-
JoinKey *joinkey = (JoinKey *) NULL;
61+
HashInfo *xhashinfo;
62+
JoinKey *joinkey;
6263

6364
xhashinfo = match_hashop_hashinfo(hashjoinop, hashinfo_list);
64-
65-
if (inner_relid == leftop->varno)
65+
joinkey = makeNode(JoinKey);
66+
if (intMember(leftop->varno, inner_relids))
6667
{
67-
joinkey = makeNode(JoinKey);
6868
joinkey->outer = rightop;
6969
joinkey->inner = leftop;
7070
}
7171
else
7272
{
73-
joinkey = makeNode(JoinKey);
7473
joinkey->outer = leftop;
7574
joinkey->inner = rightop;
7675
}
@@ -79,16 +78,15 @@ group_clauses_by_hashop(List *restrictinfo_list,
7978
{
8079
xhashinfo = makeNode(HashInfo);
8180
xhashinfo->hashop = hashjoinop;
82-
8381
xhashinfo->jmethod.jmkeys = NIL;
8482
xhashinfo->jmethod.clauses = NIL;
85-
8683
hashinfo_list = lcons(xhashinfo, hashinfo_list);
8784
}
8885

89-
xhashinfo->jmethod.clauses = lcons(clause, xhashinfo->jmethod.clauses);
90-
91-
xhashinfo->jmethod.jmkeys = lcons(joinkey, xhashinfo->jmethod.jmkeys);
86+
xhashinfo->jmethod.clauses = lcons(clause,
87+
xhashinfo->jmethod.clauses);
88+
xhashinfo->jmethod.jmkeys = lcons(joinkey,
89+
xhashinfo->jmethod.jmkeys);
9290
}
9391
}
9492
return hashinfo_list;

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.32 1999/02/22 05:26:20 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.33 1999/04/03 00:18:28 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -97,11 +97,11 @@ update_rels_pathlist_for_joins(Query *root, List *joinrels)
9797

9898
if (_enable_mergejoin_)
9999
mergeinfo_list = group_clauses_by_order(joinrel->restrictinfo,
100-
lfirsti(innerrel->relids));
100+
innerrel->relids);
101101

102102
if (_enable_hashjoin_)
103103
hashinfo_list = group_clauses_by_hashop(joinrel->restrictinfo,
104-
lfirsti(innerrel->relids));
104+
innerrel->relids);
105105

106106
/* need to flatten the relids list */
107107
joinrel->relids = nconc(listCopy(outerrelids),
@@ -173,12 +173,10 @@ best_innerjoin(List *join_paths, Relids outer_relids)
173173
{
174174
Path *path = (Path *) lfirst(join_path);
175175

176-
if (intMember(lfirsti(path->joinid), outer_relids)
177-
&& ((cheapest == NULL ||
178-
path_is_cheaper((Path *) lfirst(join_path), cheapest))))
179-
{
180-
cheapest = (Path *) lfirst(join_path);
181-
}
176+
if (intMember(lfirsti(path->joinid), outer_relids) &&
177+
(cheapest == NULL ||
178+
path_is_cheaper(path, cheapest)))
179+
cheapest = path;
182180
}
183181
return cheapest;
184182
}

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/mergeutils.c,v 1.20 1999/03/01 00:10:32 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/mergeutils.c,v 1.21 1999/04/03 00:18:28 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -36,14 +36,15 @@
3636
* Something to fix next time...
3737
*
3838
* 'restrictinfo_list' is the list of restrictinfo nodes
39-
* 'inner_relid' is the relid of the inner join relation
39+
* 'inner_relids' is the list of relids in the inner join relation
40+
* (used to determine whether a join var is inner or outer)
4041
*
4142
* Returns the new list of mergeinfo nodes.
4243
*
4344
*/
4445
List *
4546
group_clauses_by_order(List *restrictinfo_list,
46-
int inner_relid)
47+
Relids inner_relids)
4748
{
4849
List *mergeinfo_list = NIL;
4950
List *xrestrictinfo;
@@ -59,37 +60,34 @@ group_clauses_by_order(List *restrictinfo_list,
5960
* Create a new mergeinfo node and add it to 'mergeinfo_list'
6061
* if one does not yet exist for this merge ordering.
6162
*/
62-
PathOrder *pathorder;
63-
MergeInfo *xmergeinfo;
6463
Expr *clause = restrictinfo->clause;
6564
Var *leftop = get_leftop(clause);
6665
Var *rightop = get_rightop(clause);
66+
PathOrder *pathorder;
67+
MergeInfo *xmergeinfo;
6768
JoinKey *jmkeys;
6869

6970
pathorder = makeNode(PathOrder);
7071
pathorder->ordtype = MERGE_ORDER;
7172
pathorder->ord.merge = merge_ordering;
7273
xmergeinfo = match_order_mergeinfo(pathorder, mergeinfo_list);
73-
if (inner_relid == leftop->varno)
74+
jmkeys = makeNode(JoinKey);
75+
if (intMember(leftop->varno, inner_relids))
7476
{
75-
jmkeys = makeNode(JoinKey);
7677
jmkeys->outer = rightop;
7778
jmkeys->inner = leftop;
7879
}
7980
else
8081
{
81-
jmkeys = makeNode(JoinKey);
8282
jmkeys->outer = leftop;
8383
jmkeys->inner = rightop;
8484
}
8585

8686
if (xmergeinfo == NULL)
8787
{
8888
xmergeinfo = makeNode(MergeInfo);
89-
9089
xmergeinfo->m_ordering = merge_ordering;
91-
mergeinfo_list = lcons(xmergeinfo,
92-
mergeinfo_list);
90+
mergeinfo_list = lcons(xmergeinfo, mergeinfo_list);
9391
}
9492

9593
xmergeinfo->jmethod.clauses = lcons(clause,

‎src/include/optimizer/paths.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
* Copyright (c) 1994, Regents of the University of California
99
*
10-
* $Id: paths.h,v 1.26 1999/02/22 05:26:52 momjian Exp $
10+
* $Id: paths.h,v 1.27 1999/04/03 00:18:26 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -48,7 +48,7 @@ extern List *create_or_index_paths(Query *root, RelOptInfo *rel, List *clauses);
4848
* routines to deal with hash keys and clauses
4949
*/
5050
extern List *group_clauses_by_hashop(List *restrictinfo_list,
51-
int inner_relid);
51+
Relids inner_relids);
5252

5353
/*
5454
* joinutils.h
@@ -70,9 +70,9 @@ extern List *new_join_pathkeys(List *outer_pathkeys,
7070
* routines to deal with merge keys and clauses
7171
*/
7272
extern List *group_clauses_by_order(List *restrictinfo_list,
73-
int inner_relid);
73+
Relids inner_relids);
7474
extern MergeInfo *match_order_mergeinfo(PathOrder *ordering,
75-
List *mergeinfo_list);
75+
List *mergeinfo_list);
7676

7777
/*
7878
* joinrels.h

0 commit comments

Comments
 (0)