Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e54b10a

Browse files
committed
Remove the "last ditch" code path in join_search_one_level().
So far as I can tell, it is no longer possible for this heuristic to do anything useful, because the new weaker definition of have_relevant_joinclause means that any relation with a joinclause must be considered joinable to at least one other relation. It would still be possible for the code block to be entered, for example if there are join order restrictions that prevent any join of the current level from being formed; but in that case it's just a waste of cycles to attempt to form cartesian joins, since the restrictions will still apply. Furthermore, IMO the existence of this code path can mask bugs elsewhere; we would have noticed the problem with cartesian joins a lot sooner if this code hadn't compensated for it in the simplest case. Accordingly, let's remove it and see what happens. I'm committing this separately from the prerequisite changes in have_relevant_joinclause, just to make the question easier to revisit if there is some fault in my logic.
1 parent e3ffd05 commit e54b10a

File tree

1 file changed

+17
-59
lines changed

1 file changed

+17
-59
lines changed

src/backend/optimizer/path/joinrels.c

+17-59
Original file line numberDiff line numberDiff line change
@@ -169,61 +169,26 @@ join_search_one_level(PlannerInfo *root, int level)
169169
}
170170
}
171171

172-
/*
173-
* Last-ditch effort: if we failed to find any usable joins so far, force
174-
* a set of cartesian-product joins to be generated. This handles the
175-
* special case where all the available rels have join clauses but we
176-
* cannot use any of those clauses yet. An example is
172+
/*----------
173+
* Normally, we should always have made at least one join of the current
174+
* level. However, when special joins are involved, there may be no legal
175+
* way to make an N-way join for some values of N. For example consider
176+
*
177+
* SELECT ... FROM t1 WHERE
178+
* x IN (SELECT ... FROM t2,t3 WHERE ...) AND
179+
* y IN (SELECT ... FROM t4,t5 WHERE ...)
177180
*
178-
* SELECT * FROM a,b,c WHERE (a.f1 + b.f2 + c.f3) = 0;
181+
* We will flatten this query to a 5-way join problem, but there are
182+
* no 4-way joins that join_is_legal() will consider legal. We have
183+
* to accept failure at level 4 and go on to discover a workable
184+
* bushy plan at level 5.
179185
*
180-
* The join clause will be usable at level 3, but at level 2 we have no
181-
* choice but to make cartesian joins. We consider only left-sided and
182-
* right-sided cartesian joins in this case (no bushy).
186+
* However, if there are no special joins then join_is_legal() should
187+
* never fail, and so the following sanity check is useful.
188+
*----------
183189
*/
184-
if (joinrels[level] == NIL)
185-
{
186-
/*
187-
* This loop is just like the first one, except we always call
188-
* make_rels_by_clauseless_joins().
189-
*/
190-
foreach(r, joinrels[level - 1])
191-
{
192-
RelOptInfo *old_rel = (RelOptInfo *) lfirst(r);
193-
ListCell *other_rels;
194-
195-
if (level == 2)
196-
other_rels = lnext(r); /* only consider remaining initial
197-
* rels */
198-
else
199-
other_rels = list_head(joinrels[1]); /* consider all initial
200-
* rels */
201-
202-
make_rels_by_clauseless_joins(root,
203-
old_rel,
204-
other_rels);
205-
}
206-
207-
/*----------
208-
* When special joins are involved, there may be no legal way
209-
* to make an N-way join for some values of N. For example consider
210-
*
211-
* SELECT ... FROM t1 WHERE
212-
* x IN (SELECT ... FROM t2,t3 WHERE ...) AND
213-
* y IN (SELECT ... FROM t4,t5 WHERE ...)
214-
*
215-
* We will flatten this query to a 5-way join problem, but there are
216-
* no 4-way joins that join_is_legal() will consider legal. We have
217-
* to accept failure at level 4 and go on to discover a workable
218-
* bushy plan at level 5.
219-
*
220-
* However, if there are no special joins then join_is_legal() should
221-
* never fail, and so the following sanity check is useful.
222-
*----------
223-
*/
224-
if (joinrels[level] == NIL && root->join_info_list == NIL)
225-
elog(ERROR, "failed to build any %d-way joins", level);
226-
}
190+
if (joinrels[level] == NIL && root->join_info_list == NIL)
191+
elog(ERROR, "failed to build any %d-way joins", level);
227192
}
228193

229194
/*
@@ -751,13 +716,6 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
751716
* could be merged with that function, but it seems clearer to separate the
752717
* two concerns. We need this test because there are degenerate cases where
753718
* a clauseless join must be performed to satisfy join-order restrictions.
754-
*
755-
* Note: this is only a problem if one side of a degenerate outer join
756-
* contains multiple rels, or a clauseless join is required within an
757-
* IN/EXISTS RHS; else we will find a join path via the "last ditch" case in
758-
* join_search_one_level(). We could dispense with this test if we were
759-
* willing to try bushy plans in the "last ditch" case, but that seems much
760-
* less efficient.
761719
*/
762720
bool
763721
have_join_order_restriction(PlannerInfo *root,

0 commit comments

Comments
 (0)