Fix planner and rewriter to follow SQL semantics for tables that are

mentioned in FROM but not elsewhere in the query: such tables should be
joined over anyway.  Aside from being more standards-compliant, this allows
removal of some very ugly hacks for COUNT(*) processing.  Also, allow
HAVING clause without aggregate functions, since SQL does.  Clean up
CREATE RULE statement-list syntax the same way Bruce just fixed the
main stmtmulti production.
CAUTION: addition of a field to RangeTblEntry nodes breaks stored rules;
you will have to initdb if you have any rules.

src/backend/commands/view.c

@@ -5,7 +5,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- *	$Id: view.c,v 1.38 1999/10/03 23:55:27 tgl Exp $
+ *	$Id: view.c,v 1.39 1999/10/07 04:23:00 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -225,9 +225,9 @@ UpdateRangeTableOfViewParse(char *viewName, Query *viewParse)
 	 * table... CURRENT first, then NEW....
 	 */
 	rt_entry1 = addRangeTableEntry(NULL, (char *) viewName, "*CURRENT*",
-								   FALSE, FALSE);
+								   FALSE, FALSE, FALSE);
 	rt_entry2 = addRangeTableEntry(NULL, (char *) viewName, "*NEW*",
-								   FALSE, FALSE);
+								   FALSE, FALSE, FALSE);
 	new_rt = lcons(rt_entry2, old_rt);
 	new_rt = lcons(rt_entry1, new_rt);
 

src/backend/executor/execMain.c

@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.96 1999/09/29 16:06:02 wieck Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.97 1999/10/07 04:23:01 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1514,8 +1514,7 @@ ExecRelCheck(Relation rel, HeapTuple tuple, EState *estate)
 	rte->relname = nameout(&(rel->rd_rel->relname));
 	rte->refname = rte->relname;
 	rte->relid = RelationGetRelid(rel);
-	rte->inh = false;
-	rte->inFromCl = true;
+	/* inh, inFromCl, inJoinSet, skipAcl won't be used, leave them zero */
 	rtlist = lcons(rte, NIL);
 	econtext->ecxt_scantuple = slot;	/* scan tuple slot */
 	econtext->ecxt_innertuple = NULL;	/* inner tuple slot */

src/backend/nodes/copyfuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.92 1999/08/21 03:48:57 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.93 1999/10/07 04:23:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1316,9 +1316,9 @@ _copyRangeTblEntry(RangeTblEntry *from)
 	newnode->relid = from->relid;
 	newnode->inh = from->inh;
 	newnode->inFromCl = from->inFromCl;
+	newnode->inJoinSet = from->inJoinSet;
 	newnode->skipAcl = from->skipAcl;
 
-
 	return newnode;
 }
 

src/backend/nodes/equalfuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.49 1999/09/26 02:28:21 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.50 1999/10/07 04:23:04 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -597,6 +597,8 @@ _equalRangeTblEntry(RangeTblEntry *a, RangeTblEntry *b)
 		return false;
 	if (a->inFromCl != b->inFromCl)
 		return false;
+	if (a->inJoinSet != b->inJoinSet)
+		return false;
 	if (a->skipAcl != b->skipAcl)
 		return false;
 

src/backend/nodes/outfuncs.c

@@ -5,7 +5,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- *	$Id: outfuncs.c,v 1.96 1999/10/03 23:55:29 tgl Exp $
+ *	$Id: outfuncs.c,v 1.97 1999/10/07 04:23:04 tgl Exp $
  *
  * NOTES
  *	  Every (plan) node in POSTGRES has an associated "out" routine which
@@ -864,12 +864,13 @@ static void
 _outRangeTblEntry(StringInfo str, RangeTblEntry *node)
 {
 	appendStringInfo(str,
-					 " RTE :relname %s :refname %s :relid %u :inh %s :inFromCl %s :skipAcl %s",
+					 " RTE :relname %s :refname %s :relid %u :inh %s :inFromCl %s :inJoinSet %s :skipAcl %s",
 					 stringStringInfo(node->relname),
 					 stringStringInfo(node->refname),
 					 node->relid,
 					 node->inh ? "true" : "false",
 					 node->inFromCl ? "true" : "false",
+					 node->inJoinSet ? "true" : "false",
 					 node->skipAcl ? "true" : "false");
 }
 

src/backend/nodes/readfuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/readfuncs.c,v 1.73 1999/08/21 03:48:58 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/readfuncs.c,v 1.74 1999/10/07 04:23:04 tgl Exp $
  *
  * NOTES
  *	  Most of the read functions for plan nodes are tested. (In fact, they
@@ -1380,6 +1380,10 @@ _readRangeTblEntry()
 	token = lsptok(NULL, &length);		/* get :inFromCl */
 	local_node->inFromCl = (token[0] == 't') ? true : false;
 
+	token = lsptok(NULL, &length);		/* eat :inJoinSet */
+	token = lsptok(NULL, &length);		/* get :inJoinSet */
+	local_node->inJoinSet = (token[0] == 't') ? true : false;
+
 	token = lsptok(NULL, &length);		/* eat :skipAcl */
 	token = lsptok(NULL, &length);		/* get :skipAcl */
 	local_node->skipAcl = (token[0] == 't') ? true : false;

src/backend/optimizer/plan/initsplan.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.39 1999/08/26 05:07:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.40 1999/10/07 04:23:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -77,15 +77,21 @@ add_vars_to_targetlist(Query *root, List *vars)
 }
 
 /*
- * add_missing_vars_to_tlist
- *	  If we have range variable(s) in the FROM clause that does not appear
- *	  in the target list nor qualifications, we add it to the base relation
- *	  list. For instance, "select f.x from foo f, foo f2" is a join of f and
- *	  f2. Note that if we have "select foo.x from foo f", it also gets turned
- *	  into a join.
+ * add_missing_rels_to_query
+ *
+ *	  If we have a range variable in the FROM clause that does not appear
+ *	  in the target list nor qualifications, we must add it to the base
+ *	  relation list so that it will be joined.  For instance, "select f.x
+ *	  from foo f, foo f2" is a join of f and f2.  Note that if we have
+ *	  "select foo.x from foo f", it also gets turned into a join (between
+ *	  foo as foo and foo as f).
+ *
+ *	  To avoid putting useless entries into the per-relation targetlists,
+ *	  this should only be called after all the variables in the targetlist
+ *	  and quals have been processed by the routines above.
  */
 void
-add_missing_vars_to_tlist(Query *root, List *tlist)
+add_missing_rels_to_query(Query *root)
 {
 	int			varno = 1;
 	List	   *l;
@@ -93,21 +99,21 @@ add_missing_vars_to_tlist(Query *root, List *tlist)
 	foreach(l, root->rtable)
 	{
 		RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
-		Relids		relids;
 
-		relids = lconsi(varno, NIL);
-		if (rte->inFromCl && !rel_member(relids, root->base_rel_list))
+		if (rte->inJoinSet)
 		{
-			RelOptInfo *rel;
-			Var		   *var;
-
-			/* add it to base_rel_list */
-			rel = get_base_rel(root, varno);
-			/* give it a dummy tlist entry for its OID */
-			var = makeVar(varno, ObjectIdAttributeNumber, OIDOID, -1, 0);
-			add_var_to_tlist(rel, var);
+			RelOptInfo *rel = get_base_rel(root, varno);
+
+			/* If the rel isn't otherwise referenced, give it a dummy
+			 * targetlist consisting of its own OID.
+			 */
+			if (rel->targetlist == NIL)
+			{
+				Var		   *var = makeVar(varno, ObjectIdAttributeNumber,
+										  OIDOID, -1, 0);
+				add_var_to_tlist(rel, var);
+			}
 		}
-		pfree(relids);
 		varno++;
 	}
 }

src/backend/optimizer/plan/planmain.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planmain.c,v 1.45 1999/09/26 02:28:27 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planmain.c,v 1.46 1999/10/07 04:23:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,6 +28,7 @@
 
 static Plan *subplanner(Query *root, List *flat_tlist, List *qual);
 
+
 /*
  * query_planner
  *	  Routine to create a query plan.  It does so by first creating a
@@ -39,9 +40,8 @@ static Plan *subplanner(Query *root, List *flat_tlist, List *qual);
  *	  be placed where and any relation level qualifications to be
  *	  satisfied.
  *
- *	  command-type is the query command, e.g., select, delete, etc.
- *	  tlist is the target list of the query
- *	  qual is the qualification of the query
+ *	  tlist is the target list of the query (do NOT use root->targetList!)
+ *	  qual is the qualification of the query (likewise!)
  *
  *	  Note: the Query node now also includes a query_pathkeys field, which
  *	  is both an input and an output of query_planner().  The input value
@@ -57,25 +57,20 @@ static Plan *subplanner(Query *root, List *flat_tlist, List *qual);
  */
 Plan *
 query_planner(Query *root,
-			  int command_type,
 			  List *tlist,
 			  List *qual)
 {
 	List	   *constant_qual = NIL;
 	List	   *var_only_tlist;
-	List	   *level_tlist;
 	Plan	   *subplan;
 
 	/*
-	 * Simplify constant expressions in both targetlist and qual.
+	 * Note: union_planner should already have done constant folding
+	 * in both the tlist and qual, so we don't do it again here
+	 * (indeed, we may be getting a flattened var-only tlist anyway).
 	 *
-	 * Note that at this point the qual has not yet been converted to
-	 * implicit-AND form, so we can apply eval_const_expressions directly.
-	 * Also note that we need to do this before SS_process_sublinks,
-	 * because that routine inserts bogus "Const" nodes.
+	 * Is there any value in re-folding the qual after canonicalize_qual?
 	 */
-	tlist = (List *) eval_const_expressions((Node *) tlist);
-	qual = (List *) eval_const_expressions((Node *) qual);
 
 	/*
 	 * Canonicalize the qual, and convert it to implicit-AND format.
@@ -97,97 +92,75 @@ query_planner(Query *root,
 		qual = (List *) SS_process_sublinks((Node *) qual);
 
 	/*
-	 * Pull out any non-variable qualifications so these can be put in the
-	 * topmost result node.  (Any *really* non-variable quals will probably
+	 * If the query contains no relation references at all, it must be
+	 * something like "SELECT 2+2;".  Build a trivial "Result" plan.
+	 */
+	if (root->rtable == NIL)
+	{
+		/* If it's not a select, it should have had a target relation... */
+		if (root->commandType != CMD_SELECT)
+			elog(ERROR, "Empty range table for non-SELECT query");
+
+		root->query_pathkeys = NIL; /* signal unordered result */
+
+		/* Make childless Result node to evaluate given tlist. */
+		return (Plan *) make_result(tlist, (Node *) qual, (Plan *) NULL);
+	}
+
+	/*
+	 * Pull out any non-variable qual clauses so these can be put in a
+	 * toplevel "Result" node, where they will gate execution of the whole
+	 * plan (the Result will not invoke its descendant plan unless the
+	 * quals are true).  Note that any *really* non-variable quals will
 	 * have been optimized away by eval_const_expressions().  What we're
-	 * looking for here is quals that depend only on outer-level vars...)
+	 * mostly interested in here is quals that depend only on outer-level
+	 * vars, although if the qual reduces to "WHERE FALSE" this path will
+	 * also be taken.
 	 */
 	qual = pull_constant_clauses(qual, &constant_qual);
 
 	/*
 	 * Create a target list that consists solely of (resdom var) target
 	 * list entries, i.e., contains no arbitrary expressions.
+	 *
+	 * All subplan nodes will have "flat" (var-only) tlists.
+	 *
+	 * This implies that all expression evaluations are done at the root
+	 * of the plan tree.  Once upon a time there was code to try to push
+	 * expensive function calls down to lower plan nodes, but that's dead
+	 * code and has been for a long time...
 	 */
 	var_only_tlist = flatten_tlist(tlist);
-	if (var_only_tlist)
-		level_tlist = var_only_tlist;
-	else
-		/* from old code. the logic is beyond me. - ay 2/95 */
-		level_tlist = tlist;
-
-	/*
-	 * A query may have a non-variable target list and a non-variable
-	 * qualification only under certain conditions: - the query creates
-	 * all-new tuples, or - the query is a replace (a scan must still be
-	 * done in this case).
-	 */
-	if (var_only_tlist == NULL && qual == NULL)
-	{
-		root->query_pathkeys = NIL; /* these plans make unordered results */
-
-		switch (command_type)
-		{
-			case CMD_SELECT:
-			case CMD_INSERT:
-				return ((Plan *) make_result(tlist,
-											 (Node *) constant_qual,
-											 (Plan *) NULL));
-				break;
-			case CMD_DELETE:
-			case CMD_UPDATE:
-				{
-					SeqScan    *scan = make_seqscan(tlist,
-													NIL,
-													root->resultRelation);
-
-					if (constant_qual != NULL)
-						return ((Plan *) make_result(tlist,
-													 (Node *) constant_qual,
-													 (Plan *) scan));
-					else
-						return (Plan *) scan;
-				}
-				break;
-			default:
-				return (Plan *) NULL;
-		}
-	}
 
 	/*
 	 * Choose the best access path and build a plan for it.
 	 */
-	subplan = subplanner(root, level_tlist, qual);
+	subplan = subplanner(root, var_only_tlist, qual);
 
 	/*
-	 * Build a result node linking the plan if we have constant quals
+	 * Build a result node to control the plan if we have constant quals.
 	 */
 	if (constant_qual)
 	{
+		/*
+		 * The result node will also be responsible for evaluating
+		 * the originally requested tlist.
+		 */
 		subplan = (Plan *) make_result(tlist,
 									   (Node *) constant_qual,
 									   subplan);
-
-		root->query_pathkeys = NIL; /* result is unordered, no? */
-
-		return subplan;
 	}
-
-	/*
-	 * Replace the toplevel plan node's flattened target list with the
-	 * targetlist given by my caller, so that expressions are evaluated.
-	 *
-	 * This implies that all expression evaluations are done at the root
-	 * of the plan tree.  Once upon a time there was code to try to push
-	 * expensive function calls down to lower plan nodes, but that's dead
-	 * code and has been for a long time...
-	 */
 	else
 	{
+		/*
+		 * Replace the toplevel plan node's flattened target list with the
+		 * targetlist given by my caller, so that expressions are evaluated.
+		 */
 		subplan->targetlist = tlist;
-
-		return subplan;
 	}
 
+	return subplan;
+
 #ifdef NOT_USED
 
 	/*
@@ -230,12 +203,31 @@ subplanner(Query *root,
 
 	make_var_only_tlist(root, flat_tlist);
 	add_restrict_and_join_to_rels(root, qual);
-	add_missing_vars_to_tlist(root, flat_tlist);
+	add_missing_rels_to_query(root);
 
 	set_joininfo_mergeable_hashable(root->base_rel_list);
 
 	final_rel = make_one_rel(root, root->base_rel_list);
 
+	if (! final_rel)
+	{
+		/*
+		 * We expect to end up here for a trivial INSERT ... VALUES query
+		 * (which will have a target relation, so it gets past query_planner's
+		 * check for empty range table; but the target rel is unreferenced
+		 * and not marked inJoinSet, so we find there is nothing to join).
+		 * 
+		 * It's also possible to get here if the query was rewritten by the
+		 * rule processor (creating rangetable entries not marked inJoinSet)
+		 * but the rules either did nothing or were simplified to nothing
+		 * by constant-expression folding.  So, don't complain.
+		 */
+		root->query_pathkeys = NIL; /* signal unordered result */
+
+		/* Make childless Result node to evaluate given tlist. */
+		return (Plan *) make_result(flat_tlist, (Node *) qual, (Plan *) NULL);
+	}
+
 #ifdef NOT_USED					/* fix xfunc */
 
 	/*
@@ -259,13 +251,6 @@ subplanner(Query *root,
 	}
 #endif
 
-	if (! final_rel)
-	{
-		elog(NOTICE, "final relation is null");
-		root->query_pathkeys = NIL; /* result is unordered, no? */
-		return create_plan((Path *) NULL);
-	}
-
 	/*
 	 * Determine the cheapest path and create a subplan to execute it.
 	 *
@@ -344,10 +329,11 @@ subplanner(Query *root,
 		}
 	}
 
-	/* Nothing for it but to sort the cheapestpath --- but we let the
+	/*
+	 * Nothing for it but to sort the cheapestpath --- but we let the
 	 * caller do that.  union_planner has to be able to add a sort node
 	 * anyway, so no need for extra code here.  (Furthermore, the given
-	 * pathkeys might involve something we can't compute yet, such as
+	 * pathkeys might involve something we can't compute here, such as
 	 * an aggregate function...)
 	 */
 	root->query_pathkeys = final_rel->cheapestpath->pathkeys;

src/backend/optimizer/plan/planner.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.69 1999/09/26 02:28:27 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.70 1999/10/07 04:23:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -94,6 +94,37 @@ union_planner(Query *parse)
 	List	   *current_pathkeys = NIL;
 	Index		rt_index;
 
+	/*
+	 * A HAVING clause without aggregates is equivalent to a WHERE clause
+	 * (except it can only refer to grouped fields).  If there are no
+	 * aggs anywhere in the query, then we don't want to create an Agg
+	 * plan node, so merge the HAVING condition into WHERE.  (We used to
+	 * consider this an error condition, but it seems to be legal SQL.)
+	 */
+	if (parse->havingQual != NULL && ! parse->hasAggs)
+	{
+		if (parse->qual == NULL)
+			parse->qual = parse->havingQual;
+		else
+			parse->qual = (Node *) make_andclause(lappend(lcons(parse->qual,
+																NIL),
+														  parse->havingQual));
+		parse->havingQual = NULL;
+	}
+
+	/*
+	 * Simplify constant expressions in targetlist and quals.
+	 *
+	 * Note that at this point the qual has not yet been converted to
+	 * implicit-AND form, so we can apply eval_const_expressions directly.
+	 * Also note that we need to do this before SS_process_sublinks,
+	 * because that routine inserts bogus "Const" nodes.
+	 */
+	tlist = (List *) eval_const_expressions((Node *) tlist);
+	parse->qual = eval_const_expressions(parse->qual);
+	parse->havingQual = eval_const_expressions(parse->havingQual);
+
+
 	if (parse->unionClause)
 	{
 		result_plan = (Plan *) plan_union_queries(parse);
@@ -221,7 +252,6 @@ union_planner(Query *parse)
 
 		/* Generate the (sub) plan */
 		result_plan = query_planner(parse,
-									parse->commandType,
 									sub_tlist,
 									(List *) parse->qual);
 
@@ -301,25 +331,6 @@ union_planner(Query *parse)
 	 */
 	if (parse->havingQual)
 	{
-		/*--------------------
-		 * Require the havingQual to contain at least one aggregate function
-		 * (else it could have been done as a WHERE constraint).  This check
-		 * used to be much stricter, requiring an aggregate in each clause of
-		 * the CNF-ified qual.  However, that's probably overly anal-retentive.
-		 * We now do it first so that we will not complain if there is an
-		 * aggregate but it gets optimized away by eval_const_expressions().
-		 * The agg itself is never const, of course, but consider
-		 *		SELECT ... HAVING xyz OR (COUNT(*) > 1)
-		 * where xyz reduces to constant true in a particular query.
-		 * We probably should not refuse this query.
-		 *--------------------
-		 */
-		if (pull_agg_clause(parse->havingQual) == NIL)
-			elog(ERROR, "SELECT/HAVING requires aggregates to be valid");
-
-		/* Simplify constant expressions in havingQual */
-		parse->havingQual = eval_const_expressions(parse->havingQual);
-
 		/* Convert the havingQual to implicit-AND normal form */
 		parse->havingQual = (Node *)
 			canonicalize_qual((Expr *) parse->havingQual, true);

src/backend/optimizer/util/clauses.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.53 1999/10/02 04:37:52 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.54 1999/10/07 04:23:08 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -34,6 +34,11 @@
 #include "utils/syscache.h"
 
 
+/* note that pg_type.h hardwires size of bool as 1 ... duplicate it */
+#define MAKEBOOLCONST(val,isnull) \
+	((Node *) makeConst(BOOLOID, 1, (Datum) (val), \
+						(isnull), true, false, false))
+
 typedef struct {
 	List	   *groupClause;
 	List	   *targetList;
@@ -312,17 +317,20 @@ make_andclause(List *andclauses)
 }
 
 /*
- * Sometimes (such as in the result of cnfify), we use lists of expression
- * nodes with implicit AND semantics.  These functions convert between an
- * AND-semantics expression list and the ordinary representation of a
- * boolean expression.
+ * Sometimes (such as in the result of canonicalize_qual or the input of
+ * ExecQual), we use lists of expression nodes with implicit AND semantics.
+ *
+ * These functions convert between an AND-semantics expression list and the
+ * ordinary representation of a boolean expression.
+ *
+ * Note that an empty list is considered equivalent to TRUE.
  */
 Expr *
 make_ands_explicit(List *andclauses)
 {
 	if (andclauses == NIL)
-		return NULL;
-	else if (length(andclauses) == 1)
+		return (Expr *) MAKEBOOLCONST(true, false);
+	else if (lnext(andclauses) == NIL)
 		return (Expr *) lfirst(andclauses);
 	else
 		return make_andclause(andclauses);
@@ -331,10 +339,20 @@ make_ands_explicit(List *andclauses)
 List *
 make_ands_implicit(Expr *clause)
 {
+	/*
+	 * NB: because the parser sets the qual field to NULL in a query that
+	 * has no WHERE clause, we must consider a NULL input clause as TRUE,
+	 * even though one might more reasonably think it FALSE.  Grumble.
+	 * If this causes trouble, consider changing the parser's behavior.
+	 */
 	if (clause == NULL)
-		return NIL;
+		return NIL;				/* NULL -> NIL list == TRUE */
 	else if (and_clause((Node *) clause))
 		return clause->args;
+	else if (IsA(clause, Const) &&
+			 ! ((Const *) clause)->constisnull &&
+			 DatumGetInt32(((Const *) clause)->constvalue))
+		return NIL;				/* constant TRUE input -> NIL list */
 	else
 		return lcons(clause, NIL);
 }
@@ -808,11 +826,6 @@ eval_const_expressions(Node *node)
 	return eval_const_expressions_mutator(node, NULL);
 }
 
-/* note that pg_type.h hardwires size of bool as 1 ... duplicate it */
-#define MAKEBOOLCONST(val,isnull) \
-	((Node *) makeConst(BOOLOID, 1, (Datum) (val), \
-						(isnull), true, false, false))
-
 static Node *
 eval_const_expressions_mutator (Node *node, void *context)
 {

src/backend/parser/analyze.c

@@ -5,7 +5,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- *	$Id: analyze.c,v 1.120 1999/10/03 23:55:30 tgl Exp $
+ *	$Id: analyze.c,v 1.121 1999/10/07 04:23:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -298,18 +298,9 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
 
 	qry->hasSubLinks = pstate->p_hasSubLinks;
 	qry->hasAggs = pstate->p_hasAggs;
-	if (pstate->p_hasAggs || qry->groupClause)
+	if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
 		parseCheckAggregates(pstate, qry);
 
-	/*
-	 * If there is a havingQual but there are no aggregates, then there is
-	 * something wrong with the query because HAVING must contain
-	 * aggregates in its expressions! Otherwise the query could have been
-	 * formulated using the WHERE clause.
-	 */
-	if (qry->havingQual && ! qry->hasAggs)
-		elog(ERROR, "SELECT/HAVING requires aggregates to be valid");
-
 	/*
 	 * The INSERT INTO ... SELECT ... could have a UNION in child, so
 	 * unionClause may be false
@@ -961,9 +952,9 @@ transformRuleStmt(ParseState *pstate, RuleStmt *stmt)
 		nothing_qry->commandType = CMD_NOTHING;
 
 		addRangeTableEntry(pstate, stmt->object->relname, "*CURRENT*",
-						   FALSE, FALSE);
+						   FALSE, FALSE, FALSE);
 		addRangeTableEntry(pstate, stmt->object->relname, "*NEW*",
-						   FALSE, FALSE);
+						   FALSE, FALSE, FALSE);
 
 		nothing_qry->rtable = pstate->p_rtable;
 
@@ -983,9 +974,9 @@ transformRuleStmt(ParseState *pstate, RuleStmt *stmt)
 		 * equal to 2.
 		 */
 		addRangeTableEntry(pstate, stmt->object->relname, "*CURRENT*",
-						   FALSE, FALSE);
+						   FALSE, FALSE, FALSE);
 		addRangeTableEntry(pstate, stmt->object->relname, "*NEW*",
-						   FALSE, FALSE);
+						   FALSE, FALSE, FALSE);
 
 		pstate->p_last_resno = 1;
 		pstate->p_is_rule = true;		/* for expand all */
@@ -1048,18 +1039,9 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
 
 	qry->hasSubLinks = pstate->p_hasSubLinks;
 	qry->hasAggs = pstate->p_hasAggs;
-	if (pstate->p_hasAggs || qry->groupClause)
+	if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
 		parseCheckAggregates(pstate, qry);
 
-	/*
-	 * If there is a havingQual but there are no aggregates, then there is
-	 * something wrong with the query because HAVING must contain
-	 * aggregates in its expressions! Otherwise the query could have been
-	 * formulated using the WHERE clause.
-	 */
-	if (qry->havingQual && ! qry->hasAggs)
-		elog(ERROR, "SELECT/HAVING requires aggregates to be valid");
-
 	/*
 	 * The INSERT INTO ... SELECT ... could have a UNION in child, so
 	 * unionClause may be false

src/backend/parser/gram.y

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.107 1999/10/05 18:14:31 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.108 1999/10/07 04:23:12 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -72,7 +72,6 @@ static char *xlateSqlType(char *);
 static Node *makeA_Expr(int oper, char *opname, Node *lexpr, Node *rexpr);
 static Node *makeRowExpr(char *opr, List *largs, List *rargs);
 static void mapTargetColumns(List *source, List *target);
-static char *fmtId(char *rawid);
 static void param_type_init(Oid *typev, int nargs);
 static Node *doNegate(Node *n);
 
@@ -130,7 +129,7 @@ Oid	param_type(int t); /* used in parse_expr.c */
 		UpdateStmt, InsertStmt, select_clause, SelectStmt, NotifyStmt, DeleteStmt, 
 		ClusterStmt, ExplainStmt, VariableSetStmt, VariableShowStmt, VariableResetStmt,
 		CreateUserStmt, AlterUserStmt, DropUserStmt, RuleActionStmt,
-		ConstraintsSetStmt,
+		RuleActionStmtOrEmpty, ConstraintsSetStmt,
 
 %type <str>		opt_database1, opt_database2, location, encoding
 
@@ -165,7 +164,7 @@ Oid	param_type(int t); /* used in parse_expr.c */
 		result, relation_name_list, OptTableElementList,
 		OptInherit, definition,
 		opt_with, func_args, func_args_list, func_as,
-		oper_argtypes, RuleActionList, RuleActionBlock, RuleActionMulti,
+		oper_argtypes, RuleActionList, RuleActionMulti,
 		opt_column_list, columnList, opt_va_list, va_list,
 		sort_clause, sortby_list, index_params, index_list, name_list,
 		from_clause, from_expr, table_list, opt_array_bounds,
@@ -374,17 +373,18 @@ stmtblock:  stmtmulti
 				{ parsetree = $1; }
 		;
 
+/* the thrashing around here is to discard "empty" statements... */
 stmtmulti:  stmtmulti ';' stmt
-				{ if ($3 != (Node *)NIL)
+				{ if ($3 != (Node *)NULL)
 					$$ = lappend($1, $3);
 				  else
 					$$ = $1;
 				}
 		| stmt
-				{ if ($1 != (Node *)NIL)
+				{ if ($1 != (Node *)NULL)
 					$$ = lcons($1,NIL);
 				  else
-					$$ = (Node *)NIL;
+					$$ = NIL;
 				}
 		;
 
@@ -433,7 +433,7 @@ stmt :	  AddAttrStmt
 		| VariableResetStmt
 		| ConstraintsSetStmt
 		|	/*EMPTY*/
-				{ $$ = (Node *)NIL; }
+				{ $$ = (Node *)NULL; }
 		;
 
 /*****************************************************************************
@@ -930,7 +930,7 @@ ColConstraint:
 		CONSTRAINT name ColConstraintElem
 				{
 						Constraint *n = (Constraint *)$3;
-						if (n != NULL) n->name = fmtId($2);
+						if (n != NULL) n->name = $2;
 						$$ = $3;
 				}
 		| ColConstraintElem
@@ -1024,7 +1024,7 @@ ColConstraintElem:  CHECK '(' a_expr ')'
 TableConstraint:  CONSTRAINT name ConstraintElem
 				{
 						Constraint *n = (Constraint *)$3;
-						if (n != NULL) n->name = fmtId($2);
+						if (n != NULL) n->name = $2;
 						$$ = $3;
 				}
 		| ConstraintElem
@@ -2034,20 +2034,23 @@ RuleStmt:  CREATE RULE name AS
 RuleActionList:  NOTHING				{ $$ = NIL; }
 		| SelectStmt					{ $$ = lcons($1, NIL); }
 		| RuleActionStmt				{ $$ = lcons($1, NIL); }
-		| '[' RuleActionBlock ']'		{ $$ = $2; }
-		| '(' RuleActionBlock ')'		{ $$ = $2; } 
-		;
-
-RuleActionBlock:  RuleActionMulti		{  $$ = $1; }
-		| RuleActionStmt				{ $$ = lcons($1, NIL); }
+		| '[' RuleActionMulti ']'		{ $$ = $2; }
+		| '(' RuleActionMulti ')'		{ $$ = $2; } 
 		;
 
-RuleActionMulti:  RuleActionMulti RuleActionStmt
-				{  $$ = lappend($1, $2); }
-		| RuleActionMulti RuleActionStmt ';'
-				{  $$ = lappend($1, $2); }
-		| RuleActionStmt ';'
-				{ $$ = lcons($1, NIL); }
+/* the thrashing around here is to discard "empty" statements... */
+RuleActionMulti:  RuleActionMulti ';' RuleActionStmtOrEmpty
+				{ if ($3 != (Node *)NULL)
+					$$ = lappend($1, $3);
+				  else
+					$$ = $1;
+				}
+		| RuleActionStmtOrEmpty
+				{ if ($1 != (Node *)NULL)
+					$$ = lcons($1,NIL);
+				  else
+					$$ = NIL;
+				}
 		;
 
 RuleActionStmt:	InsertStmt
@@ -2056,6 +2059,11 @@ RuleActionStmt:	InsertStmt
 		| NotifyStmt
 		;
 
+RuleActionStmtOrEmpty:	RuleActionStmt
+		|	/*EMPTY*/
+				{ $$ = (Node *)NULL; }
+		;
+
 event_object:  relation_name '.' attr_name
 				{
 					$$ = makeNode(Attr);
@@ -3676,12 +3684,25 @@ a_expr:  attr
 				{	$$ = makeA_Expr(OP, $2, $1, NULL); }
 		| func_name '(' '*' ')'
 				{
-					/* cheap hack for aggregate (eg. count) */
+					/*
+					 * For now, we transform AGGREGATE(*) into AGGREGATE(1).
+					 *
+					 * This does the right thing for COUNT(*) (in fact,
+					 * any certainly-non-null expression would do for COUNT),
+					 * and there are no other aggregates in SQL92 that accept
+					 * '*' as parameter.
+					 *
+					 * XXX really, the '*' ought to be transformed to some
+					 * special construct that wouldn't be acceptable as the
+					 * input of a non-aggregate function, in case the given
+					 * func_name matches a plain function.  This would also
+					 * support a possible extension to let user-defined
+					 * aggregates do something special with '*' as input.
+					 */
 					FuncCall *n = makeNode(FuncCall);
 					A_Const *star = makeNode(A_Const);
-
-					star->val.type = T_String;
-					star->val.val.str = "";
+					star->val.type = T_Integer;
+					star->val.val.ival = 1;
 					n->funcname = $1;
 					n->args = lcons(star, NIL);
 					$$ = (Node *)n;
@@ -5265,30 +5286,6 @@ void parser_init(Oid *typev, int nargs)
 }
 
 
-/* fmtId()
- * Check input string for non-lowercase/non-numeric characters.
- * Returns either input string or input surrounded by double quotes.
- */
-static char *
-fmtId(char *rawid)
-{
-	static char *cp;
-
-	for (cp = rawid; *cp != '\0'; cp++)
-		if (! (islower(*cp) || isdigit(*cp) || (*cp == '_'))) break;
-
-	if (*cp != '\0') {
-		cp = palloc(strlen(rawid)+3);
-		strcpy(cp,"\"");
-		strcat(cp,rawid);
-		strcat(cp,"\"");
-	} else {
-		cp = rawid;
-	};
-
-	return cp;
-}
-
 /*
  * param_type_init()
  *
@@ -5313,6 +5310,11 @@ Oid param_type(int t)
  *	The optimizer doesn't like '-' 4 for index use.  It only checks for
  *	Var '=' Const.  It wants an integer of -4, so we try to merge the
  *	minus into the constant.
+ *
+ *	This code is no longer essential as of 10/1999, since the optimizer
+ *	now has a constant-subexpression simplifier.  However, we can save
+ *	a few cycles throughout the parse and rewrite stages if we collapse
+ *	the minus into the constant sooner rather than later...
  */
 static Node *doNegate(Node *n)
 {

src/backend/parser/parse_agg.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_agg.c,v 1.28 1999/08/21 03:48:55 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_agg.c,v 1.29 1999/10/07 04:23:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -129,8 +129,8 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
 	List	   *groupClauses = NIL;
 	List	   *tl;
 
-	/* This should only be called if we found aggregates or grouping */
-	Assert(pstate->p_hasAggs || qry->groupClause);
+	/* This should only be called if we found aggregates, GROUP, or HAVING */
+	Assert(pstate->p_hasAggs || qry->groupClause || qry->havingQual);
 
 	/*
 	 * Aggregates must never appear in WHERE clauses. (Note this check
@@ -160,6 +160,15 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
 		groupClauses = lcons(expr, groupClauses);
 	}
 
+	/*
+	 * The expression specified in the HAVING clause can only contain
+	 * aggregates, group columns and functions thereof.  As with WHERE,
+	 * we want to point the finger at HAVING before the target list.
+	 */
+	if (!exprIsAggOrGroupCol(qry->havingQual, groupClauses))
+		elog(ERROR,
+			 "Illegal use of aggregates or non-group column in HAVING clause");
+
 	/*
 	 * The target list can only contain aggregates, group columns and
 	 * functions thereof.
@@ -173,14 +182,6 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
 				 "Illegal use of aggregates or non-group column in target list");
 	}
 
-	/*
-	 * The expression specified in the HAVING clause has the same
-	 * restriction as those in the target list.
-	 */
-	if (!exprIsAggOrGroupCol(qry->havingQual, groupClauses))
-		elog(ERROR,
-			 "Illegal use of aggregates or non-group column in HAVING clause");
-
 	/* Release the list storage (but not the pointed-to expressions!) */
 	freeList(groupClauses);
 }
@@ -190,12 +191,12 @@ Aggref *
 ParseAgg(ParseState *pstate, char *aggname, Oid basetype,
 		 List *target, int precedence)
 {
+	HeapTuple	theAggTuple;
+	Form_pg_aggregate aggform;
 	Oid			fintype;
-	Oid			vartype;
 	Oid			xfn1;
-	Form_pg_aggregate aggform;
+	Oid			vartype;
 	Aggref	   *aggref;
-	HeapTuple	theAggTuple;
 	bool		usenulls = false;
 
 	theAggTuple = SearchSysCacheTuple(AGGNAME,
@@ -206,66 +207,19 @@ ParseAgg(ParseState *pstate, char *aggname, Oid basetype,
 		elog(ERROR, "Aggregate %s does not exist", aggname);
 
 	/*
-	 * We do a major hack for count(*) here.
-	 *
-	 * Count(*) poses several problems.  First, we need a field that is
-	 * guaranteed to be in the range table, and unique.  Using a constant
-	 * causes the optimizer to properly remove the aggragate from any
-	 * elements of the query. Using just 'oid', which can not be null, in
-	 * the parser fails on:
+	 * There used to be a really ugly hack for count(*) here.
 	 *
-	 * select count(*) from tab1, tab2	   -- oid is not unique select
-	 * count(*) from viewtable		-- views don't have real oids
+	 * It's gone.  Now, the grammar transforms count(*) into count(1),
+	 * which does the right thing.  (It didn't use to do the right thing,
+	 * because the optimizer had the wrong ideas about semantics of queries
+	 * without explicit variables.  Fixed as of Oct 1999 --- tgl.)
 	 *
-	 * So, for an aggregate with parameter '*', we use the first valid range
-	 * table entry, and pick the first column from the table. We set a
-	 * flag to count nulls, because we could have nulls in that column.
-	 *
-	 * It's an ugly job, but someone has to do it. bjm 1998/1/18
+	 * Since "1" never evaluates as null, we currently have no need of
+	 * the "usenulls" flag, but it should be kept around; in fact, we should
+	 * extend the pg_aggregate table to let usenulls be specified as an
+	 * attribute of user-defined aggregates.
 	 */
 
-	if (nodeTag(lfirst(target)) == T_Const)
-	{
-		Const	   *con = (Const *) lfirst(target);
-
-		if (con->consttype == UNKNOWNOID && VARSIZE(con->constvalue) == VARHDRSZ)
-		{
-			Attr	   *attr = makeNode(Attr);
-			List	   *rtable,
-					   *rlist;
-			RangeTblEntry *first_valid_rte;
-
-			Assert(lnext(target) == NULL);
-
-			if (pstate->p_is_rule)
-				rtable = lnext(lnext(pstate->p_rtable));
-			else
-				rtable = pstate->p_rtable;
-
-			first_valid_rte = NULL;
-			foreach(rlist, rtable)
-			{
-				RangeTblEntry *rte = lfirst(rlist);
-
-				/* only entries on outer(non-function?) scope */
-				if (!rte->inFromCl && rte != pstate->p_target_rangetblentry)
-					continue;
-
-				first_valid_rte = rte;
-				break;
-			}
-			if (first_valid_rte == NULL)
-				elog(ERROR, "Can't find column to do aggregate(*) on.");
-
-			attr->relname = first_valid_rte->refname;
-			attr->attrs = lcons(makeString(
-						   get_attname(first_valid_rte->relid, 1)), NIL);
-
-			lfirst(target) = transformExpr(pstate, (Node *) attr, precedence);
-			usenulls = true;
-		}
-	}
-
 	aggform = (Form_pg_aggregate) GETSTRUCT(theAggTuple);
 	fintype = aggform->aggfinaltype;
 	xfn1 = aggform->aggtransfn1;

src/backend/parser/parse_clause.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.45 1999/09/18 19:07:12 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.46 1999/10/07 04:23:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -60,6 +60,15 @@ makeRangeTable(ParseState *pstate, List *frmList, Node **qual)
  * setTargetTable
  *	  Add the target relation of INSERT or UPDATE to the range table,
  *	  and make the special links to it in the ParseState.
+ *
+ *	  Note that the target is not marked as either inFromCl or inJoinSet.
+ *	  For INSERT, we don't want the target to be joined to; it's a
+ *	  destination of tuples, not a source.  For UPDATE/DELETE, we do
+ *	  need to scan or join the target.  This will happen without the
+ *	  inJoinSet flag because the planner's preprocess_targetlist()
+ *	  adds the destination's CTID attribute to the targetlist, and
+ *	  therefore the destination will be a referenced table even if
+ *	  there is no other use of any of its attributes.  Tricky, eh?
  */
 void
 setTargetTable(ParseState *pstate, char *relname)
@@ -69,7 +78,8 @@ setTargetTable(ParseState *pstate, char *relname)
 
 	if ((refnameRangeTablePosn(pstate, relname, &sublevels_up) == 0)
 		|| (sublevels_up != 0))
-		rte = addRangeTableEntry(pstate, relname, relname, FALSE, FALSE);
+		rte = addRangeTableEntry(pstate, relname, relname,
+								 FALSE, FALSE, FALSE);
 	else
 		rte = refnameRangeTableEntry(pstate, relname);
 
@@ -230,7 +240,8 @@ transformTableEntry(ParseState *pstate, RangeVar *r)
 	 * we expand * to foo.x.
 	 */
 
-	rte = addRangeTableEntry(pstate, relname, refname, baserel->inh, TRUE);
+	rte = addRangeTableEntry(pstate, relname, refname,
+							 baserel->inh, TRUE, TRUE);
 
 	return refname;
 }

src/backend/parser/parse_func.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.59 1999/09/29 18:16:04 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.60 1999/10/07 04:23:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -274,7 +274,8 @@ ParseFuncOrColumn(ParseState *pstate, char *funcname, List *fargs,
 			rte = refnameRangeTableEntry(pstate, refname);
 			if (rte == NULL)
 			{
-				rte = addRangeTableEntry(pstate, refname, refname,FALSE, FALSE);
+				rte = addRangeTableEntry(pstate, refname, refname,
+										 FALSE, FALSE, TRUE);
 #ifdef WARN_FROM
 				elog(NOTICE,"Adding missing FROM-clause entry%s for table %s",
 					pstate->parentParseState != NULL ? " in subquery" : "",
@@ -437,7 +438,8 @@ ParseFuncOrColumn(ParseState *pstate, char *funcname, List *fargs,
 			rte = refnameRangeTableEntry(pstate, refname);
 			if (rte == NULL)
 			{
-				rte = addRangeTableEntry(pstate, refname, refname,FALSE, FALSE);
+				rte = addRangeTableEntry(pstate, refname, refname,
+										 FALSE, FALSE, TRUE);
 #ifdef WARN_FROM
 				elog(NOTICE,"Adding missing FROM-clause entry%s for table %s",
 					pstate->parentParseState != NULL ? " in subquery" : "",

src/backend/parser/parse_relation.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.31 1999/09/29 18:16:04 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.32 1999/10/07 04:23:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -144,7 +144,7 @@ colnameRangeTableEntry(ParseState *pstate, char *colname)
 		{
 			RangeTblEntry *rte = lfirst(et);
 
-			/* only entries on outer(non-function?) scope */
+			/* only consider RTEs mentioned in FROM or UPDATE/DELETE */
 			if (!rte->inFromCl && rte != pstate->p_target_rangetblentry)
 				continue;
 
@@ -178,45 +178,51 @@ addRangeTableEntry(ParseState *pstate,
 				   char *relname,
 				   char *refname,
 				   bool inh,
-				   bool inFromCl)
+				   bool inFromCl,
+				   bool inJoinSet)
 {
 	Relation	relation;
-	RangeTblEntry *rte = makeNode(RangeTblEntry);
+	RangeTblEntry *rte;
 	int			sublevels_up;
 
 	if (pstate != NULL)
 	{
-		if (refnameRangeTablePosn(pstate, refname, &sublevels_up) != 0 &&
-			(!inFromCl || sublevels_up == 0))
+		int			rt_index = refnameRangeTablePosn(pstate, refname,
+													 &sublevels_up);
+
+		if (rt_index != 0 && (!inFromCl || sublevels_up == 0))
 		{
 			if (!strcmp(refname, "*CURRENT*") || !strcmp(refname, "*NEW*"))
-			{
-				int			rt_index = refnameRangeTablePosn(pstate, refname, &sublevels_up);
-
 				return (RangeTblEntry *) nth(rt_index - 1, pstate->p_rtable);
-			}
 			elog(ERROR, "Table name '%s' specified more than once", refname);
 		}
 	}
 
+	rte = makeNode(RangeTblEntry);
+
 	rte->relname = pstrdup(relname);
 	rte->refname = pstrdup(refname);
 
+	/* Get the rel's OID.  This access also ensures that we have an
+	 * up-to-date relcache entry for the rel.  We don't need to keep
+	 * it open, however.
+	 */
 	relation = heap_openr(relname, AccessShareLock);
 	rte->relid = RelationGetRelid(relation);
 	heap_close(relation, AccessShareLock);
 
 	/*
-	 * Flags - zero or more from inheritance,union,version or recursive
-	 * (transitive closure) [we don't support them all -- ay 9/94 ]
+	 * Flags: this RTE should be expanded to include descendant tables,
+	 * this RTE is in the FROM clause, this RTE should be included in
+	 * the planner's final join.
 	 */
 	rte->inh = inh;
-
-	/* RelOID */
 	rte->inFromCl = inFromCl;
+	rte->inJoinSet = inJoinSet;
+	rte->skipAcl = false;		/* always starts out false */
 
 	/*
-	 * close the relation we're done with it for now.
+	 * Add completed RTE to range table list.
 	 */
 	if (pstate != NULL)
 		pstate->p_rtable = lappend(pstate->p_rtable, rte);
@@ -240,7 +246,8 @@ expandAll(ParseState *pstate, char *relname, char *refname, int *this_resno)
 	rte = refnameRangeTableEntry(pstate, refname);
 	if (rte == NULL)
 	{
-		rte = addRangeTableEntry(pstate, relname, refname, FALSE, FALSE);
+		rte = addRangeTableEntry(pstate, relname, refname,
+								 FALSE, FALSE, TRUE);
 #ifdef WARN_FROM
 		elog(NOTICE,"Adding missing FROM-clause entry%s for table %s",
 			pstate->parentParseState != NULL ? " in subquery" : "",

src/backend/rewrite/rewriteHandler.c

@@ -6,7 +6,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.59 1999/10/02 04:42:04 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.60 1999/10/07 04:23:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -818,12 +818,13 @@ ApplyRetrieveRule(Query *parsetree,
 				  int rt_index,
 				  int relation_level,
 				  Relation relation,
+				  bool relWasInJoinSet,
 				  int *modified)
 {
 	Query	   *rule_action = NULL;
 	Node	   *rule_qual;
 	List	   *rtable,
-			   *rt,
+			   *addedrtable,
 			   *l;
 	int			nothing,
 				rt_length;
@@ -844,19 +845,23 @@ ApplyRetrieveRule(Query *parsetree,
 		nothing = TRUE;
 
 	rtable = copyObject(parsetree->rtable);
-	foreach(rt, rtable)
-	{
-		RangeTblEntry *rte = lfirst(rt);
+	rt_length = length(rtable);	/* original length, not counting rule */
 
-		/*
-		 * this is to prevent add_missing_vars_to_base_rels() from adding
-		 * a bogus entry to the new target list.
-		 */
-		rte->inFromCl = false;
+	addedrtable = copyObject(rule_action->rtable);
+
+	/* If the original rel wasn't in the join set, none of its spawn is.
+	 * If it was, then leave the spawn's flags as they are.
+	 */
+	if (! relWasInJoinSet)
+	{
+		foreach(l, addedrtable)
+		{
+			RangeTblEntry *rte = lfirst(l);
+			rte->inJoinSet = false;
+		}
 	}
-	rt_length = length(rtable);
 
-	rtable = nconc(rtable, copyObject(rule_action->rtable));
+	rtable = nconc(rtable, addedrtable);
 	parsetree->rtable = rtable;
 
 	/* FOR UPDATE of view... */
@@ -1006,10 +1011,14 @@ fireRIRrules(Query *parsetree)
 	RuleLock   *rules;
 	RewriteRule *rule;
 	RewriteRule RIRonly;
+	bool		relWasInJoinSet;
 	int			modified = false;
 	int			i;
 	List	   *l;
 
+	/* don't try to convert this into a foreach loop, because
+	 * rtable list can get changed each time through...
+	 */
 	rt_index = 0;
 	while (rt_index < length(parsetree->rtable))
 	{
@@ -1017,46 +1026,57 @@ fireRIRrules(Query *parsetree)
 
 		rte = nth(rt_index - 1, parsetree->rtable);
 
-		if (!rangeTableEntry_used((Node *) parsetree, rt_index, 0))
+		/*
+		 * If the table is not one named in the original FROM clause
+		 * then it must be referenced in the query, or we ignore it.
+		 * This prevents infinite expansion loop due to new rtable
+		 * entries inserted by expansion of a rule.
+		 */
+		if (! rte->inFromCl && rt_index != parsetree->resultRelation &&
+			! rangeTableEntry_used((Node *) parsetree, rt_index, 0))
 		{
-
-			/*
-			 * Unused range table entries must not be marked as coming
-			 * from a clause. Otherwise the planner will generate joins
-			 * over relations that in fact shouldn't be scanned at all and
-			 * the result will contain duplicates
-			 *
-			 * Jan
-			 *
-			 */
-			rte->inFromCl = FALSE;
+			/* Make sure the planner ignores it too... */
+			rte->inJoinSet = false;
 			continue;
 		}
 
 		rel = heap_openr(rte->relname, AccessShareLock);
-		if (rel->rd_rules == NULL)
+		rules = rel->rd_rules;
+		if (rules == NULL)
 		{
 			heap_close(rel, AccessShareLock);
 			continue;
 		}
 
-		rules = rel->rd_rules;
-		locks = NIL;
+		relWasInJoinSet = rte->inJoinSet; /* save before possibly clearing */
 
 		/*
 		 * Collect the RIR rules that we must apply
 		 */
+		locks = NIL;
 		for (i = 0; i < rules->numLocks; i++)
 		{
 			rule = rules->rules[i];
 			if (rule->event != CMD_SELECT)
 				continue;
 
-			if (rule->attrno > 0 &&
-				!attribute_used((Node *) parsetree,
-								rt_index,
-								rule->attrno, 0))
-				continue;
+			if (rule->attrno > 0)
+			{
+				/* per-attr rule; do we need it? */
+				if (! attribute_used((Node *) parsetree,
+									 rt_index,
+									 rule->attrno, 0))
+					continue;
+			}
+			else
+			{
+				/* Rel-wide ON SELECT DO INSTEAD means this is a view.
+				 * Remove the view from the planner's join target set,
+				 * or we'll get no rows out because view itself is empty!
+				 */
+				if (rule->isInstead)
+					rte->inJoinSet = false;
+			}
 
 			locks = lappend(locks, rule);
 		}
@@ -1083,6 +1103,7 @@ fireRIRrules(Query *parsetree)
 										  rt_index,
 										  RIRonly.attrno == -1,
 										  rel,
+										  relWasInJoinSet,
 										  &modified);
 		}
 
@@ -2012,10 +2033,10 @@ Except_Intersect_Rewrite(Query *parsetree)
 			 * If the Select Query node has aggregates in use add all the
 			 * subselects to the HAVING qual else to the WHERE qual
 			 */
-			if (intersect_node->hasAggs == false)
-				AddQual(intersect_node, (Node *) n);
-			else
+			if (intersect_node->hasAggs)
 				AddHavingQual(intersect_node, (Node *) n);
+			else
+				AddQual(intersect_node, (Node *) n);
 
 			/* Now we got sublinks */
 			intersect_node->hasSubLinks = true;

src/include/nodes/parsenodes.h

@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: parsenodes.h,v 1.83 1999/10/03 23:55:36 tgl Exp $
+ * $Id: parsenodes.h,v 1.84 1999/10/07 04:23:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -45,7 +45,7 @@ typedef struct Query
 	bool		isBinary;		/* binary portal? */
 	bool		isTemp;			/* is 'into' a temp table? */
 	bool		unionall;		/* union without unique sort */
-	bool		hasAggs;		/* has aggregates in target list */
+	bool		hasAggs;		/* has aggregates in tlist or havingQual */
 	bool		hasSubLinks;	/* has subquery SubLink */
 
 	List	   *rtable;			/* list of range table entries */
@@ -968,30 +968,48 @@ typedef struct TargetEntry
 	NodeTag		type;
 	Resdom	   *resdom;			/* fjoin overload this to be a list?? */
 	Fjoin	   *fjoin;
-	Node	   *expr;			/* can be a list too */
+	Node	   *expr;
 } TargetEntry;
 
-/*
+/*--------------------
  * RangeTblEntry -
- *	  used in range tables. Some of the following are only used in one of
+ *	  A range table is a List of RangeTblEntry nodes.
+ *
+ *	  Some of the following are only used in one of
  *	  the parsing, optimizing, execution stages.
  *
- *	  inFromCl marks those range variables that are listed in the from clause.
- *	  In SQL, the targetlist can only refer to range variables listed in the
- *	  from clause but POSTQUEL allows you to refer to tables not specified, in
- *	  which case a range table entry will be generated. We use POSTQUEL
- *	  semantics which is more powerful. However, we need SQL semantics in
- *	  some cases (eg. when expanding a '*')
+ *	  inFromCl marks those range variables that are listed in the FROM clause.
+ *	  In SQL, the query can only refer to range variables listed in the
+ *	  FROM clause, but POSTQUEL allows you to refer to tables not listed,
+ *	  in which case a range table entry will be generated.  We still support
+ *	  this POSTQUEL feature, although there is some doubt whether it's
+ *	  convenient or merely confusing.  The flag is needed since an
+ *	  implicitly-added RTE shouldn't change the namespace for unqualified
+ *	  column names processed later, and it also shouldn't affect the
+ *	  expansion of '*'.
+ *
+ *	  inJoinSet marks those range variables that the planner should join
+ *	  over even if they aren't explicitly referred to in the query.  For
+ *	  example, "SELECT COUNT(1) FROM tx" should produce the number of rows
+ *	  in tx.  A more subtle example uses a POSTQUEL implicit RTE:
+ *			SELECT COUNT(1) FROM tx WHERE TRUE OR (tx.f1 = ty.f2)
+ *	  Here we should get the product of the sizes of tx and ty.  However,
+ *	  the query optimizer can simplify the WHERE clause to "TRUE", so
+ *	  ty will no longer be referred to explicitly; without a flag forcing
+ *	  it to be included in the join, we will get the wrong answer.  So,
+ *	  a POSTQUEL implicit RTE must be marked inJoinSet but not inFromCl.
+ *--------------------
  */
 typedef struct RangeTblEntry
 {
 	NodeTag		type;
 	char	   *relname;		/* real name of the relation */
-	char	   *refname;		/* the reference name (specified in the
-								 * from clause) */
-	Oid			relid;
-	bool		inh;			/* inheritance? */
-	bool		inFromCl;		/* comes from From Clause */
+	char	   *refname;		/* the reference name (as specified in the
+								 * FROM clause) */
+	Oid			relid;			/* OID of the relation */
+	bool		inh;			/* inheritance requested? */
+	bool		inFromCl;		/* present in FROM clause */
+	bool		inJoinSet;		/* planner must include this rel */
 	bool		skipAcl;		/* skip ACL check in executor */
 } RangeTblEntry;
 

src/include/optimizer/planmain.h

@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: planmain.h,v 1.33 1999/08/22 23:56:43 tgl Exp $
+ * $Id: planmain.h,v 1.34 1999/10/07 04:23:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,9 +19,7 @@
 /*
  * prototypes for plan/planmain.c
  */
-extern Plan *query_planner(Query *root,
-			  int command_type, List *tlist, List *qual);
-
+extern Plan *query_planner(Query *root, List *tlist, List *qual);
 
 /*
  * prototypes for plan/createplan.c
@@ -42,8 +40,8 @@ extern Result *make_result(List *tlist, Node *resconstantqual, Plan *subplan);
  */
 extern void make_var_only_tlist(Query *root, List *tlist);
 extern void add_restrict_and_join_to_rels(Query *root, List *clauses);
+extern void add_missing_rels_to_query(Query *root);
 extern void set_joininfo_mergeable_hashable(List *rel_list);
-extern void add_missing_vars_to_tlist(Query *root, List *tlist);
 
 /*
  * prototypes for plan/setrefs.c

src/include/parser/parse_relation.h

@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: parse_relation.h,v 1.12 1999/07/19 00:26:17 tgl Exp $
+ * $Id: parse_relation.h,v 1.13 1999/10/07 04:23:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -23,7 +23,8 @@ extern RangeTblEntry *addRangeTableEntry(ParseState *pstate,
 				   char *relname,
 				   char *refname,
 				   bool inh,
-				   bool inFromCl);
+				   bool inFromCl,
+				   bool inJoinSet);
 extern List *expandAll(ParseState *pstate, char *relname, char *refname,
 		  int *this_resno);
 extern int	attnameAttNum(Relation rd, char *a);

src/test/regress/expected/rules.out

@@ -1075,9 +1075,9 @@ pg_user           |SELECT pg_shadow.usename, pg_shadow.usesysid, pg_shadow.usecr
 pg_views          |SELECT c.relname AS viewname, pg_get_userbyid(c.relowner) AS viewowner, pg_get_viewdef(c.relname) AS definition FROM pg_class c WHERE (c.relhasrules AND (EXISTS (SELECT r.rulename FROM pg_rewrite r WHERE ((r.ev_class = c.oid) AND (r.ev_type = '1'::char)))));                                                                                                                           
 rtest_v1          |SELECT rtest_t1.a, rtest_t1.b FROM rtest_t1;                                                                                                                                                                                                                                                                                                                                                 
 rtest_vcomp       |SELECT x.part, (x.size * y.factor) AS size_in_cm FROM rtest_comp x, rtest_unitfact y WHERE (x.unit = y.unit);                                                                                                                                                                                                                                                                                
-rtest_vview1      |SELECT x.a, x.b FROM rtest_view1 x WHERE (0 < (SELECT count(y.a) AS count FROM rtest_view2 y WHERE (y.a = x.a)));                                                                                                                                                                                                                                                                            
+rtest_vview1      |SELECT x.a, x.b FROM rtest_view1 x WHERE (0 < (SELECT count(1) AS count FROM rtest_view2 y WHERE (y.a = x.a)));                                                                                                                                                                                                                                                                              
 rtest_vview2      |SELECT rtest_view1.a, rtest_view1.b FROM rtest_view1 WHERE rtest_view1.v;                                                                                                                                                                                                                                                                                                                    
-rtest_vview3      |SELECT x.a, x.b FROM rtest_vview2 x WHERE (0 < (SELECT count(y.a) AS count FROM rtest_view2 y WHERE (y.a = x.a)));                                                                                                                                                                                                                                                                           
+rtest_vview3      |SELECT x.a, x.b FROM rtest_vview2 x WHERE (0 < (SELECT count(1) AS count FROM rtest_view2 y WHERE (y.a = x.a)));                                                                                                                                                                                                                                                                             
 rtest_vview4      |SELECT x.a, x.b, count(y.a) AS refcount FROM rtest_view1 x, rtest_view2 y WHERE (x.a = y.a) GROUP BY x.a, x.b;                                                                                                                                                                                                                                                                               
 rtest_vview5      |SELECT rtest_view1.a, rtest_view1.b, rtest_viewfunc1(rtest_view1.a) AS refcount FROM rtest_view1;                                                                                                                                                                                                                                                                                            
 shoe              |SELECT sh.shoename, sh.sh_avail, sh.slcolor, sh.slminlen, (sh.slminlen * un.un_fact) AS slminlen_cm, sh.slmaxlen, (sh.slmaxlen * un.un_fact) AS slmaxlen_cm, sh.slunit FROM shoe_data sh, unit un WHERE (sh.slunit = un.un_name);