Clean up messy clause-selectivity code in clausesel.c; repair bug

identified by Hiroshi (incorrect cost attributed to OR clauses
after multiple passes through set_rest_selec()).  I think the code
was trying to allow selectivities of OR subclauses to be passed in
from outside, but noplace was actually passing any useful data, and
set_rest_selec() was passing wrong data.

Restructure representation of "indexqual" in IndexPath nodes so that
it is the same as for indxqual in completed IndexScan nodes: namely,
a toplevel list with an entry for each pass of the index scan, having
sublists that are implicitly-ANDed index qual conditions for that pass.
You don't want to know what the old representation was :-(

Improve documentation of OR-clause indexscan functions.

Remove useless 'notclause' field from RestrictInfo nodes.  (This might
force an initdb for anyone who has stored rules containing RestrictInfos,
but I do not think that RestrictInfo ever appears in completed plans.)

1 files changed, 46 insertions, 75 deletions

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 2270a0e4eab..08ed6a73163 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.34 1999/07/16 04:59:19 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.35 1999/07/24 23:21:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -25,12 +25,11 @@
 #include "optimizer/var.h"
 #include "utils/lsyscache.h"
 
-extern int	Quiet;
 
-static void add_restrict_and_join_to_rel(Query *root, List *clause);
+static void add_restrict_and_join_to_rel(Query *root, Node *clause);
 static void add_join_info_to_rels(Query *root, RestrictInfo *restrictinfo,
 					  Relids join_relids);
-static void add_vars_to_targetlist(Query *root, List *vars, Relids join_relids);
+static void add_vars_to_targetlist(Query *root, List *vars);
 
 static MergeOrder *mergejoinop(Expr *clause);
 static Oid	hashjoinop(Expr *clause);
@@ -129,7 +128,7 @@ add_missing_vars_to_tlist(Query *root, List *tlist)
  *	  relations appearing within clauses.  Creates new relation entries if
  *	  necessary, adding them to *query_relation_list*.
  *
- *	  Returns nothing of interest.
+ * 'clauses': the list of clauses in the cnfify'd query qualification.
  */
 void
 add_restrict_and_join_to_rels(Query *root, List *clauses)
@@ -137,95 +136,71 @@ add_restrict_and_join_to_rels(Query *root, List *clauses)
 	List	   *clause;
 
 	foreach(clause, clauses)
-		add_restrict_and_join_to_rel(root, lfirst(clause));
-	return;
+		add_restrict_and_join_to_rel(root, (Node*) lfirst(clause));
 }
 
 /*
  * add_restrict_and_join_to_rel-
  *	  Add clause information to either the 'RestrictInfo' or 'JoinInfo' field
- *	  of a relation entry(depending on whether or not the clause is a join)
+ *	  of a relation entry (depending on whether or not the clause is a join)
  *	  by creating a new RestrictInfo node and setting appropriate fields
  *	  within the nodes.
- *
- *	  Returns nothing of interest.
  */
 static void
-add_restrict_and_join_to_rel(Query *root, List *clause)
+add_restrict_and_join_to_rel(Query *root, Node *clause)
 {
+	RestrictInfo *restrictinfo = makeNode(RestrictInfo);
 	Relids		relids;
 	List	   *vars;
-	RestrictInfo *restrictinfo = makeNode(RestrictInfo);
-
-	/*
-	 * Retrieve all relids and vars contained within the clause.
-	 */
-	clause_get_relids_vars((Node *) clause, &relids, &vars);
 
 	restrictinfo->clause = (Expr *) clause;
-	restrictinfo->notclause = contains_not((Node *) clause);
-	restrictinfo->selectivity = 0;
 	restrictinfo->indexids = NIL;
 	restrictinfo->mergejoinorder = (MergeOrder *) NULL;
 	restrictinfo->hashjoinoperator = (Oid) 0;
 
+	/*
+	 * The selectivity of the clause must be computed regardless of
+	 * whether it's a restriction or a join clause
+	 */
+	restrictinfo->selectivity = compute_clause_selec(root, clause);
+
+	/*
+	 * Retrieve all relids and vars contained within the clause.
+	 */
+	clause_get_relids_vars(clause, &relids, &vars);
+
 	if (length(relids) == 1)
 	{
-
 		/*
 		 * There is only one relation participating in 'clause', so
-		 * 'clause' must be a restriction clause.
+		 * 'clause' must be a restriction clause for that relation.
 		 */
 		RelOptInfo *rel = get_base_rel(root, lfirsti(relids));
 
-		/*
-		 * The selectivity of the clause must be computed regardless of
-		 * whether it's a restriction or a join clause
-		 */
-		if (is_funcclause((Node *) clause))
-
-			/*
-			 * XXX If we have a func clause set selectivity to 1/3, really
-			 * need a true selectivity function.
-			 */
-			restrictinfo->selectivity = (Cost) 0.3333333;
-		else
-			restrictinfo->selectivity = compute_clause_selec(root, (Node *) clause, NIL);
-
 		rel->restrictinfo = lcons(restrictinfo, rel->restrictinfo);
 	}
 	else
 	{
-
 		/*
 		 * 'clause' is a join clause, since there is more than one atom in
-		 * the relid list.
+		 * the relid list.  Add it to the join lists of all the relevant
+		 * relations.  (If, perchance, 'clause' contains NO vars, then
+		 * nothing will happen...)
 		 */
-		if (is_funcclause((Node *) clause))
-
-			/*
-			 * XXX If we have a func clause set selectivity to 1/3, really
-			 * need a true selectivity function.
-			 */
-			restrictinfo->selectivity = (Cost) 0.3333333;
-		else
-			restrictinfo->selectivity = compute_clause_selec(root, (Node *) clause, NIL);
-
 		add_join_info_to_rels(root, restrictinfo, relids);
-		/* we are going to be doing a join, so add var to targetlist */
-		add_vars_to_targetlist(root, vars, relids);
+		/* we are going to be doing a join, so add vars to targetlists */
+		add_vars_to_targetlist(root, vars);
 	}
 }
 
 /*
  * add_join_info_to_rels
  *	  For every relation participating in a join clause, add 'restrictinfo' to
- *	  the appropriate joininfo node(creating a new one and adding it to the
+ *	  the appropriate joininfo node (creating a new one and adding it to the
  *	  appropriate rel node if necessary).
  *
  * 'restrictinfo' describes the join clause
  * 'join_relids' is the list of relations participating in the join clause
- *
  */
 static void
 add_join_info_to_rels(Query *root, RestrictInfo *restrictinfo,
@@ -233,7 +208,7 @@ add_join_info_to_rels(Query *root, RestrictInfo *restrictinfo,
 {
 	List	   *join_relid;
 
-	/* For every relid, find the rel, and add the proper join entries */
+	/* For every relid, find the joininfo, and add the proper join entries */
 	foreach(join_relid, join_relids)
 	{
 		JoinInfo   *joininfo;
@@ -247,43 +222,39 @@ add_join_info_to_rels(Query *root, RestrictInfo *restrictinfo,
 				unjoined_relids = lappendi(unjoined_relids, lfirsti(rel));
 		}
 
+		/*
+		 * Find or make the joininfo node for this combination of rels
+		 */
 		joininfo = find_joininfo_node(get_base_rel(root, lfirsti(join_relid)),
 									  unjoined_relids);
+
+		/*
+		 * And add the restrictinfo node to it.  NOTE that each joininfo
+		 * gets its own copy of the restrictinfo node!  (Is this really
+		 * necessary?  Possibly ... later parts of the optimizer destructively
+		 * modify restrict/join clauses...)
+		 */
 		joininfo->jinfo_restrictinfo = lcons(copyObject((void *) restrictinfo),
-										   joininfo->jinfo_restrictinfo);
+											 joininfo->jinfo_restrictinfo);
 	}
 }
 
 /*
  * add_vars_to_targetlist
- *	  For each variable appearing in a clause,
- *	  (1) If a targetlist entry for the variable is not already present in
- *		  the appropriate relation's target list, add one.
- *	  (2) If a targetlist entry is already present, but the var is part of a
- *		  join clause, add the relids of the join relations to the JoinList
- *		  entry of the targetlist entry.
- *
- *	  'vars' is the list of var nodes
- *	  'join_relids' is the list of relids appearing in the join clause
- *		(if this is a join clause)
- *
- *	  Returns nothing.
+ *	  For each variable appearing in a clause, add it to the relation's
+ *	  targetlist if not already present.
  */
 static void
-add_vars_to_targetlist(Query *root, List *vars, Relids join_relids)
+add_vars_to_targetlist(Query *root, List *vars)
 {
-	Var		   *var;
-	List	   *temp = NIL;
-	RelOptInfo *rel = (RelOptInfo *) NULL;
-	TargetEntry *tlistentry;
+	List	   *temp;
 
 	foreach(temp, vars)
 	{
-		var = (Var *) lfirst(temp);
-		rel = get_base_rel(root, var->varno);
-		tlistentry = tlistentry_member(var, rel->targetlist);
-		if (tlistentry == NULL)
-			/* add a new entry */
+		Var		   *var = (Var *) lfirst(temp);
+		RelOptInfo *rel = get_base_rel(root, var->varno);
+
+		if (tlistentry_member(var, rel->targetlist) == NULL)
 			add_var_to_tlist(rel, var);
 	}
 }