From: Tom Lane Date: Thu, 7 Oct 1999 04:23:24 +0000 (+0000) Subject: Fix planner and rewriter to follow SQL semantics for tables that are X-Git-Tag: REL7_0~1358 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3eb1c8227751aecede58e742a13b07127a7e2652;p=postgresql 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. --- diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index a88b1840dd..65c46d271a 100644 --- a/src/backend/commands/view.c +++ b/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); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 0943e4085b..fed4666aee 100644 --- a/src/backend/executor/execMain.c +++ b/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 */ diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 4895be7237..07e75c0f83 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/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; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index d948fb1e44..2e3d67da60 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/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; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 6b1e560014..06c2520d27 100644 --- a/src/backend/nodes/outfuncs.c +++ b/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"); } diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 588528daa1..ef62e5a285 100644 --- a/src/backend/nodes/readfuncs.c +++ b/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; diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index 20a01902ef..ef219245aa 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.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++; } } diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c index 3cc3f0a694..3b289fb3d0 100644 --- a/src/backend/optimizer/plan/planmain.c +++ b/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; diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index d78a650c05..fce3800dc4 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/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); diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 1fc91f0cae..066b391826 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/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) { diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 4bcf79a727..08c209b2a5 100644 --- a/src/backend/parser/analyze.c +++ b/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 diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 006e545c3a..a88b66c970 100644 --- a/src/backend/parser/gram.y +++ b/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 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) { diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index a500782297..5c87d5bc85 100644 --- a/src/backend/parser/parse_agg.c +++ b/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; diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index bae53ebbd8..81f3f88669 100644 --- a/src/backend/parser/parse_clause.c +++ b/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; } diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index ca56e59204..c6f9610699 100644 --- a/src/backend/parser/parse_func.c +++ b/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" : "", diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index e664615e6d..fcbd6fedd2 100644 --- a/src/backend/parser/parse_relation.c +++ b/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" : "", diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 4ad87f16ba..ca9a2c27d8 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/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; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 8ac9e3d764..1556bf3869 100644 --- a/src/include/nodes/parsenodes.h +++ b/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; diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h index 09c39ceea7..8664fb9aaa 100644 --- a/src/include/optimizer/planmain.h +++ b/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 diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index 68e5ac7bf1..2f2305263a 100644 --- a/src/include/parser/parse_relation.h +++ b/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); diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 685233132d..3889862425 100644 --- a/src/test/regress/expected/rules.out +++ b/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);