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

Commit eb919e8

Browse files
committed
Resurrect the "last ditch" code path in join_search_one_level().
This essentially reverts commit e54b10a, in which I'd decided that the "last ditch" join logic was useless. The folly of that is now exposed by a report from Pavel Stehule: although the function should always find at least one join in a self-contained join problem, it can still fail to do so in a sub-problem created by artificial from_collapse_limit or join_collapse_limit constraints. Adjust the comments to describe this, and simplify the code a bit to match the new coding of the earlier loop in the function. I'm not terribly happy about this: I still subscribe to the opinion stated in the previous commit message that the "last ditch" code can obscure logic bugs elsewhere. But the alternative seems to be to complicate the earlier tests for does-this-relation-have-a-join-clause to the point where they can tell whether the join clauses link outside the current join sub-problem. And that looks messy, slow, and possibly a source of bugs in itself. In any case, now is not the time to be inserting experimental code into 9.2, so let's just go back to the time-tested solution.
1 parent 864de65 commit eb919e8

File tree

1 file changed

+56
-15
lines changed

1 file changed

+56
-15
lines changed

src/backend/optimizer/path/joinrels.c

+56-15
Original file line numberDiff line numberDiff line change
@@ -178,25 +178,59 @@ join_search_one_level(PlannerInfo *root, int level)
178178
}
179179

180180
/*----------
181-
* Normally, we should always have made at least one join of the current
182-
* level. However, when special joins are involved, there may be no legal
183-
* way to make an N-way join for some values of N. For example consider
181+
* Last-ditch effort: if we failed to find any usable joins so far, force
182+
* a set of cartesian-product joins to be generated. This handles the
183+
* special case where all the available rels have join clauses but we
184+
* cannot use any of those clauses yet. This can only happen when we are
185+
* considering a join sub-problem (a sub-joinlist) and all the rels in the
186+
* sub-problem have only join clauses with rels outside the sub-problem.
187+
* An example is
184188
*
185-
* SELECT ... FROM t1 WHERE
186-
* x IN (SELECT ... FROM t2,t3 WHERE ...) AND
187-
* y IN (SELECT ... FROM t4,t5 WHERE ...)
189+
* SELECT ... FROM a INNER JOIN b ON TRUE, c, d, ...
190+
* WHERE a.w = c.x and b.y = d.z;
188191
*
189-
* We will flatten this query to a 5-way join problem, but there are
190-
* no 4-way joins that join_is_legal() will consider legal. We have
191-
* to accept failure at level 4 and go on to discover a workable
192-
* bushy plan at level 5.
193-
*
194-
* However, if there are no special joins then join_is_legal() should
195-
* never fail, and so the following sanity check is useful.
192+
* If the "a INNER JOIN b" sub-problem does not get flattened into the
193+
* upper level, we must be willing to make a cartesian join of a and b;
194+
* but the code above will not have done so, because it thought that both
195+
* a and b have joinclauses. We consider only left-sided and right-sided
196+
* cartesian joins in this case (no bushy).
196197
*----------
197198
*/
198-
if (joinrels[level] == NIL && root->join_info_list == NIL)
199-
elog(ERROR, "failed to build any %d-way joins", level);
199+
if (joinrels[level] == NIL)
200+
{
201+
/*
202+
* This loop is just like the first one, except we always call
203+
* make_rels_by_clauseless_joins().
204+
*/
205+
foreach(r, joinrels[level - 1])
206+
{
207+
RelOptInfo *old_rel = (RelOptInfo *) lfirst(r);
208+
209+
make_rels_by_clauseless_joins(root,
210+
old_rel,
211+
list_head(joinrels[1]));
212+
}
213+
214+
/*----------
215+
* When special joins are involved, there may be no legal way
216+
* to make an N-way join for some values of N. For example consider
217+
*
218+
* SELECT ... FROM t1 WHERE
219+
* x IN (SELECT ... FROM t2,t3 WHERE ...) AND
220+
* y IN (SELECT ... FROM t4,t5 WHERE ...)
221+
*
222+
* We will flatten this query to a 5-way join problem, but there are
223+
* no 4-way joins that join_is_legal() will consider legal. We have
224+
* to accept failure at level 4 and go on to discover a workable
225+
* bushy plan at level 5.
226+
*
227+
* However, if there are no special joins then join_is_legal() should
228+
* never fail, and so the following sanity check is useful.
229+
*----------
230+
*/
231+
if (joinrels[level] == NIL && root->join_info_list == NIL)
232+
elog(ERROR, "failed to build any %d-way joins", level);
233+
}
200234
}
201235

202236
/*
@@ -724,6 +758,13 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
724758
* could be merged with that function, but it seems clearer to separate the
725759
* two concerns. We need this test because there are degenerate cases where
726760
* a clauseless join must be performed to satisfy join-order restrictions.
761+
*
762+
* Note: this is only a problem if one side of a degenerate outer join
763+
* contains multiple rels, or a clauseless join is required within an
764+
* IN/EXISTS RHS; else we will find a join path via the "last ditch" case in
765+
* join_search_one_level(). We could dispense with this test if we were
766+
* willing to try bushy plans in the "last ditch" case, but that seems much
767+
* less efficient.
727768
*/
728769
bool
729770
have_join_order_restriction(PlannerInfo *root,

0 commit comments

Comments
 (0)