Index: src/backend/optimizer/README
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/README,v
retrieving revision 1.35.2.1
diff -c -r1.35.2.1 README
*** src/backend/optimizer/README	13 Feb 2007 02:31:11 -0000	1.35.2.1
--- src/backend/optimizer/README	26 Oct 2007 17:54:03 -0000
***************
*** 172,178 ****
  If the query contains one-sided outer joins (LEFT or RIGHT joins), or
  "IN (sub-select)" WHERE clauses that were converted to joins, then some of
  the possible join orders may be illegal.  These are excluded by having
! make_join_rel consult side lists of outer joins and IN joins to see
  whether a proposed join is illegal.  (The same consultation allows it
  to see which join style should be applied for a valid join, ie,
  JOIN_INNER, JOIN_LEFT, etc.)
--- 172,178 ----
  If the query contains one-sided outer joins (LEFT or RIGHT joins), or
  "IN (sub-select)" WHERE clauses that were converted to joins, then some of
  the possible join orders may be illegal.  These are excluded by having
! join_is_legal consult side lists of outer joins and IN joins to see
  whether a proposed join is illegal.  (The same consultation allows it
  to see which join style should be applied for a valid join, ie,
  JOIN_INNER, JOIN_LEFT, etc.)
***************
*** 215,221 ****
  translating the jointree to "joinlist" representation.  LEFT and RIGHT
  JOIN nodes are normally collapsed so that they participate fully in the
  join order search.  To avoid generating illegal join orders, the planner
! creates an OuterJoinInfo node for each outer join, and make_join_rel
  checks this list to decide if a proposed join is legal.
  
  What we store in OuterJoinInfo nodes are the minimum sets of Relids
--- 215,221 ----
  translating the jointree to "joinlist" representation.  LEFT and RIGHT
  JOIN nodes are normally collapsed so that they participate fully in the
  join order search.  To avoid generating illegal join orders, the planner
! creates an OuterJoinInfo node for each outer join, and join_is_legal
  checks this list to decide if a proposed join is legal.
  
  What we store in OuterJoinInfo nodes are the minimum sets of Relids
***************
*** 224,230 ****
  rels to the OJ's syntactic rels may be legal.  Per identities 1 and 2,
  non-FULL joins can be freely associated into the lefthand side of an
  OJ, but in general they can't be associated into the righthand side.
! So the restriction enforced by make_join_rel is that a proposed join
  can't join a rel within or partly within an RHS boundary to one outside
  the boundary, unless the join validly implements some outer join.
  (To support use of identity 3, we have to allow cases where an apparent
--- 224,230 ----
  rels to the OJ's syntactic rels may be legal.  Per identities 1 and 2,
  non-FULL joins can be freely associated into the lefthand side of an
  OJ, but in general they can't be associated into the righthand side.
! So the restriction enforced by join_is_legal is that a proposed join
  can't join a rel within or partly within an RHS boundary to one outside
  the boundary, unless the join validly implements some outer join.
  (To support use of identity 3, we have to allow cases where an apparent
Index: src/backend/optimizer/path/joinrels.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/path/joinrels.c,v
retrieving revision 1.81.2.3
diff -c -r1.81.2.3 joinrels.c
*** src/backend/optimizer/path/joinrels.c	16 Feb 2007 00:14:07 -0000	1.81.2.3
--- src/backend/optimizer/path/joinrels.c	26 Oct 2007 17:54:03 -0000
***************
*** 26,31 ****
--- 26,32 ----
  							  RelOptInfo *old_rel,
  							  ListCell *other_rels);
  static bool has_join_restriction(PlannerInfo *root, RelOptInfo *rel);
+ static bool has_legal_joinclause(PlannerInfo *root, RelOptInfo *rel);
  
  
  /*
***************
*** 222,232 ****
  		 *	 y IN (SELECT ... FROM t4,t5 WHERE ...)
  		 *
  		 * We will flatten this query to a 5-way join problem, but there are
! 		 * no 4-way joins that make_join_rel() will consider legal.  We have
  		 * to accept failure at level 4 and go on to discover a workable
  		 * bushy plan at level 5.
  		 *
! 		 * However, if there are no such clauses then make_join_rel() should
  		 * never fail, and so the following sanity check is useful.
  		 *----------
  		 */
--- 223,233 ----
  		 *	 y IN (SELECT ... FROM t4,t5 WHERE ...)
  		 *
  		 * We will flatten this query to a 5-way join problem, but there are
! 		 * no 4-way joins that join_is_legal() will consider legal.  We have
  		 * to accept failure at level 4 and go on to discover a workable
  		 * bushy plan at level 5.
  		 *
! 		 * However, if there are no such clauses then join_is_legal() should
  		 * never fail, and so the following sanity check is useful.
  		 *----------
  		 */
***************
*** 325,356 ****
  
  
  /*
!  * make_join_rel
!  *	   Find or create a join RelOptInfo that represents the join of
!  *	   the two given rels, and add to it path information for paths
!  *	   created with the two rels as outer and inner rel.
!  *	   (The join rel may already contain paths generated from other
!  *	   pairs of rels that add up to the same set of base rels.)
   *
!  * NB: will return NULL if attempted join is not valid.  This can happen
!  * when working with outer joins, or with IN clauses that have been turned
!  * into joins.
   */
! RelOptInfo *
! make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
  {
- 	Relids		joinrelids;
  	JoinType	jointype;
  	bool		is_valid_inner;
- 	RelOptInfo *joinrel;
- 	List	   *restrictlist;
  	ListCell   *l;
  
! 	/* We should never try to join two overlapping sets of rels. */
! 	Assert(!bms_overlap(rel1->relids, rel2->relids));
! 
! 	/* Construct Relids set that identifies the joinrel. */
! 	joinrelids = bms_union(rel1->relids, rel2->relids);
  
  	/*
  	 * If we have any outer joins, the proposed join might be illegal; and in
--- 326,354 ----
  
  
  /*
!  * join_is_legal
!  *	   Determine whether a proposed join is legal given the query's
!  *	   join order constraints; and if it is, determine the join type.
   *
!  * Caller must supply not only the two rels, but the union of their relids.
!  * (We could simplify the API by computing joinrelids locally, but this
!  * would be redundant work in the normal path through make_join_rel.)
!  *
!  * On success, *jointype_p is set to the required join type.
   */
! static bool
! join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
! 			  Relids joinrelids, JoinType *jointype_p)
  {
  	JoinType	jointype;
  	bool		is_valid_inner;
  	ListCell   *l;
  
! 	/*
! 	 * Ensure *jointype_p is set on failure return.  This is just to
! 	 * suppress uninitialized-variable warnings from overly anal compilers.
! 	 */
! 	*jointype_p = JOIN_INNER;
  
  	/*
  	 * If we have any outer joins, the proposed join might be illegal; and in
***************
*** 399,420 ****
  			bms_is_subset(ojinfo->min_righthand, rel2->relids))
  		{
  			if (jointype != JOIN_INNER)
! 			{
! 				/* invalid join path */
! 				bms_free(joinrelids);
! 				return NULL;
! 			}
  			jointype = ojinfo->is_full_join ? JOIN_FULL : JOIN_LEFT;
  		}
  		else if (bms_is_subset(ojinfo->min_lefthand, rel2->relids) &&
  				 bms_is_subset(ojinfo->min_righthand, rel1->relids))
  		{
  			if (jointype != JOIN_INNER)
! 			{
! 				/* invalid join path */
! 				bms_free(joinrelids);
! 				return NULL;
! 			}
  			jointype = ojinfo->is_full_join ? JOIN_FULL : JOIN_RIGHT;
  		}
  		else
--- 397,410 ----
  			bms_is_subset(ojinfo->min_righthand, rel2->relids))
  		{
  			if (jointype != JOIN_INNER)
! 				return false;			/* invalid join path */
  			jointype = ojinfo->is_full_join ? JOIN_FULL : JOIN_LEFT;
  		}
  		else if (bms_is_subset(ojinfo->min_lefthand, rel2->relids) &&
  				 bms_is_subset(ojinfo->min_righthand, rel1->relids))
  		{
  			if (jointype != JOIN_INNER)
! 				return false;			/* invalid join path */
  			jointype = ojinfo->is_full_join ? JOIN_FULL : JOIN_RIGHT;
  		}
  		else
***************
*** 457,467 ****
  
  	/* Fail if violated some OJ's RHS and didn't match to another OJ */
  	if (jointype == JOIN_INNER && !is_valid_inner)
! 	{
! 		/* invalid join path */
! 		bms_free(joinrelids);
! 		return NULL;
! 	}
  
  	/*
  	 * Similarly, if we are implementing IN clauses as joins, check for
--- 447,453 ----
  
  	/* Fail if violated some OJ's RHS and didn't match to another OJ */
  	if (jointype == JOIN_INNER && !is_valid_inner)
! 		return false;			/* invalid join path */
  
  	/*
  	 * Similarly, if we are implementing IN clauses as joins, check for
***************
*** 493,502 ****
  		 * subselect.
  		 */
  		if (!bms_is_subset(ininfo->righthand, joinrelids))
! 		{
! 			bms_free(joinrelids);
! 			return NULL;
! 		}
  
  		/*
  		 * At this point we are considering a join of the IN's RHS to some
--- 479,485 ----
  		 * subselect.
  		 */
  		if (!bms_is_subset(ininfo->righthand, joinrelids))
! 			return false;
  
  		/*
  		 * At this point we are considering a join of the IN's RHS to some
***************
*** 524,533 ****
  		 * that needs to trigger here.
  		 */
  		if (jointype != JOIN_INNER)
! 		{
! 			bms_free(joinrelids);
! 			return NULL;
! 		}
  		if (bms_is_subset(ininfo->lefthand, rel1->relids) &&
  			bms_equal(ininfo->righthand, rel2->relids))
  			jointype = JOIN_IN;
--- 507,513 ----
  		 * that needs to trigger here.
  		 */
  		if (jointype != JOIN_INNER)
! 			return false;
  		if (bms_is_subset(ininfo->lefthand, rel1->relids) &&
  			bms_equal(ininfo->righthand, rel2->relids))
  			jointype = JOIN_IN;
***************
*** 539,549 ****
  		else if (bms_equal(ininfo->righthand, rel2->relids))
  			jointype = JOIN_UNIQUE_INNER;
  		else
! 		{
! 			/* invalid join path */
! 			bms_free(joinrelids);
! 			return NULL;
! 		}
  	}
  
  	/*
--- 519,565 ----
  		else if (bms_equal(ininfo->righthand, rel2->relids))
  			jointype = JOIN_UNIQUE_INNER;
  		else
! 			return false;			/* invalid join path */
! 	}
! 
! 	/* Join is valid */
! 	*jointype_p = jointype;
! 	return true;
! }
! 
! 
! /*
!  * make_join_rel
!  *	   Find or create a join RelOptInfo that represents the join of
!  *	   the two given rels, and add to it path information for paths
!  *	   created with the two rels as outer and inner rel.
!  *	   (The join rel may already contain paths generated from other
!  *	   pairs of rels that add up to the same set of base rels.)
!  *
!  * NB: will return NULL if attempted join is not valid.  This can happen
!  * when working with outer joins, or with IN clauses that have been turned
!  * into joins.
!  */
! RelOptInfo *
! make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
! {
! 	Relids		joinrelids;
! 	JoinType	jointype;
! 	RelOptInfo *joinrel;
! 	List	   *restrictlist;
! 
! 	/* We should never try to join two overlapping sets of rels. */
! 	Assert(!bms_overlap(rel1->relids, rel2->relids));
! 
! 	/* Construct Relids set that identifies the joinrel. */
! 	joinrelids = bms_union(rel1->relids, rel2->relids);
! 
! 	/* Check validity and determine join type. */
! 	if (!join_is_legal(root, rel1, rel2, joinrelids, &jointype))
! 	{
! 		/* invalid join path */
! 		bms_free(joinrelids);
! 		return NULL;
  	}
  
  	/*
***************
*** 645,650 ****
--- 661,667 ----
  have_join_order_restriction(PlannerInfo *root,
  							RelOptInfo *rel1, RelOptInfo *rel2)
  {
+ 	bool		result = false;
  	ListCell   *l;
  
  	/*
***************
*** 666,675 ****
  		/* Can we perform the OJ with these rels? */
  		if (bms_is_subset(ojinfo->min_lefthand, rel1->relids) &&
  			bms_is_subset(ojinfo->min_righthand, rel2->relids))
! 			return true;
  		if (bms_is_subset(ojinfo->min_lefthand, rel2->relids) &&
  			bms_is_subset(ojinfo->min_righthand, rel1->relids))
! 			return true;
  
  		/*
  		 * Might we need to join these rels to complete the RHS?  We have
--- 683,698 ----
  		/* Can we perform the OJ with these rels? */
  		if (bms_is_subset(ojinfo->min_lefthand, rel1->relids) &&
  			bms_is_subset(ojinfo->min_righthand, rel2->relids))
! 		{
! 			result = true;
! 			break;
! 		}
  		if (bms_is_subset(ojinfo->min_lefthand, rel2->relids) &&
  			bms_is_subset(ojinfo->min_righthand, rel1->relids))
! 		{
! 			result = true;
! 			break;
! 		}
  
  		/*
  		 * Might we need to join these rels to complete the RHS?  We have
***************
*** 678,689 ****
  		 */
  		if (bms_overlap(ojinfo->min_righthand, rel1->relids) &&
  			bms_overlap(ojinfo->min_righthand, rel2->relids))
! 			return true;
  
  		/* Likewise for the LHS. */
  		if (bms_overlap(ojinfo->min_lefthand, rel1->relids) &&
  			bms_overlap(ojinfo->min_lefthand, rel2->relids))
! 			return true;
  	}
  
  	/*
--- 701,718 ----
  		 */
  		if (bms_overlap(ojinfo->min_righthand, rel1->relids) &&
  			bms_overlap(ojinfo->min_righthand, rel2->relids))
! 		{
! 			result = true;
! 			break;
! 		}
  
  		/* Likewise for the LHS. */
  		if (bms_overlap(ojinfo->min_lefthand, rel1->relids) &&
  			bms_overlap(ojinfo->min_lefthand, rel2->relids))
! 		{
! 			result = true;
! 			break;
! 		}
  	}
  
  	/*
***************
*** 697,706 ****
  		/* Can we perform the IN with these rels? */
  		if (bms_is_subset(ininfo->lefthand, rel1->relids) &&
  			bms_is_subset(ininfo->righthand, rel2->relids))
! 			return true;
  		if (bms_is_subset(ininfo->lefthand, rel2->relids) &&
  			bms_is_subset(ininfo->righthand, rel1->relids))
! 			return true;
  
  		/*
  		 * Might we need to join these rels to complete the RHS?  It's
--- 726,741 ----
  		/* Can we perform the IN with these rels? */
  		if (bms_is_subset(ininfo->lefthand, rel1->relids) &&
  			bms_is_subset(ininfo->righthand, rel2->relids))
! 		{
! 			result = true;
! 			break;
! 		}
  		if (bms_is_subset(ininfo->lefthand, rel2->relids) &&
  			bms_is_subset(ininfo->righthand, rel1->relids))
! 		{
! 			result = true;
! 			break;
! 		}
  
  		/*
  		 * Might we need to join these rels to complete the RHS?  It's
***************
*** 710,724 ****
  		 */
  		if (bms_overlap(ininfo->righthand, rel1->relids) &&
  			bms_overlap(ininfo->righthand, rel2->relids))
! 			return true;
  
  		/* Likewise for the LHS. */
  		if (bms_overlap(ininfo->lefthand, rel1->relids) &&
  			bms_overlap(ininfo->lefthand, rel2->relids))
! 			return true;
  	}
  
! 	return false;
  }
  
  
--- 745,781 ----
  		 */
  		if (bms_overlap(ininfo->righthand, rel1->relids) &&
  			bms_overlap(ininfo->righthand, rel2->relids))
! 		{
! 			result = true;
! 			break;
! 		}
  
  		/* Likewise for the LHS. */
  		if (bms_overlap(ininfo->lefthand, rel1->relids) &&
  			bms_overlap(ininfo->lefthand, rel2->relids))
! 		{
! 			result = true;
! 			break;
! 		}
  	}
  
! 	/*
! 	 * We do not force the join to occur if either input rel can legally
! 	 * be joined to anything else using joinclauses.  This essentially
! 	 * means that clauseless bushy joins are put off as long as possible.
! 	 * The reason is that when there is a join order restriction high up
! 	 * in the join tree (that is, with many rels inside the LHS or RHS),
! 	 * we would otherwise expend lots of effort considering very stupid
! 	 * join combinations within its LHS or RHS.
! 	 */
! 	if (result)
! 	{
! 		if (has_legal_joinclause(root, rel1) ||
! 			has_legal_joinclause(root, rel2))
! 			result = false;
! 	}
! 
! 	return result;
  }
  
  
***************
*** 728,734 ****
   *		due to being inside an outer join or an IN (sub-SELECT).
   *
   * Essentially, this tests whether have_join_order_restriction() could
!  * succeed with this rel and some other one.
   */
  static bool
  has_join_restriction(PlannerInfo *root, RelOptInfo *rel)
--- 785,793 ----
   *		due to being inside an outer join or an IN (sub-SELECT).
   *
   * Essentially, this tests whether have_join_order_restriction() could
!  * succeed with this rel and some other one.  It's OK if we sometimes
!  * say "true" incorrectly.  (Therefore, we don't bother with the relatively
!  * expensive has_legal_joinclause test.)
   */
  static bool
  has_join_restriction(PlannerInfo *root, RelOptInfo *rel)
***************
*** 771,773 ****
--- 830,890 ----
  
  	return false;
  }
+ 
+ 
+ /*
+  * has_legal_joinclause
+  *		Detect whether the specified relation can legally be joined
+  *		to any other rels using join clauses.
+  *
+  * We consider only joins to single other relations.  This is sufficient
+  * to get a "true" result in most real queries, and an occasional erroneous
+  * "false" will only cost a bit more planning time.  The reason for this
+  * limitation is that considering joins to other joins would require proving
+  * that the other join rel can legally be formed, which seems like too much
+  * trouble for something that's only a heuristic to save planning time.
+  */
+ static bool
+ has_legal_joinclause(PlannerInfo *root, RelOptInfo *rel)
+ {
+ 	Index		rti;
+ 
+ 	for (rti = 1; rti < root->simple_rel_array_size; rti++)
+ 	{
+ 		RelOptInfo *rel2 = root->simple_rel_array[rti];
+ 
+ 		/* there may be empty slots corresponding to non-baserel RTEs */
+ 		if (rel2 == NULL)
+ 			continue;
+ 
+ 		Assert(rel2->relid == rti);		/* sanity check on array */
+ 
+ 		/* ignore RTEs that are "other rels" */
+ 		if (rel2->reloptkind != RELOPT_BASEREL)
+ 			continue;
+ 
+ 		/* ignore RTEs that are already in "rel" */
+ 		if (bms_overlap(rel->relids, rel2->relids))
+ 			continue;
+ 
+ 		if (have_relevant_joinclause(root, rel, rel2))
+ 		{
+ 			Relids		joinrelids;
+ 			JoinType	jointype;
+ 
+ 			/* join_is_legal needs relids of the union */
+ 			joinrelids = bms_union(rel->relids, rel2->relids);
+ 
+ 			if (join_is_legal(root, rel, rel2, joinrelids, &jointype))
+ 			{
+ 				/* Yes, this will work */
+ 				bms_free(joinrelids);
+ 				return true;
+ 			}
+ 
+ 			bms_free(joinrelids);
+ 		}
+ 	}
+ 
+ 	return false;
+ }
