From e64a331179607fece8cfaef4f2a3ffb6a481f4f2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 16 Mar 2000 03:23:18 +0000 Subject: [PATCH] Fix some (more) problems with subselects in rules. Rewriter failed to mark query as having subselects if a subselect was added from a rule WHERE condition (as opposed to a rule action). Also, fix adjustment of varlevelsup so that it actually has some prospect of working when inserting an expression containing a subselect into a subquery. --- src/backend/rewrite/rewriteHandler.c | 168 ++++--------------------- src/backend/rewrite/rewriteManip.c | 179 ++++++++++++++++++++++++--- src/include/rewrite/rewriteManip.h | 7 +- 3 files changed, 188 insertions(+), 166 deletions(-) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 0ba42f0594..ec22f1e6dd 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.68 2000/03/12 18:57:05 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.69 2000/03/16 03:23:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -55,16 +55,11 @@ static RewriteInfo *gatherRewriteMeta(Query *parsetree, static bool rangeTableEntry_used(Node *node, int rt_index, int sublevels_up); static bool attribute_used(Node *node, int rt_index, int attno, int sublevels_up); -static bool modifyAggrefUplevel(Node *node, void *context); static bool modifyAggrefChangeVarnodes(Node *node, int rt_index, int new_index, int sublevels_up, int new_sublevels_up); static Node *modifyAggrefDropQual(Node *node, Node *targetNode); static SubLink *modifyAggrefMakeSublink(Aggref *aggref, Query *parsetree); static Node *modifyAggrefQual(Node *node, Query *parsetree); -static bool checkQueryHasAggs(Node *node); -static bool checkQueryHasAggs_walker(Node *node, void *context); -static bool checkQueryHasSubLink(Node *node); -static bool checkQueryHasSubLink_walker(Node *node, void *context); static Query *fireRIRrules(Query *parsetree); static Query *Except_Intersect_Rewrite(Query *parsetree); static void check_targetlists_are_compatible(List *prev_target, @@ -290,65 +285,15 @@ attribute_used(Node *node, int rt_index, int attno, int sublevels_up) } -/* - * modifyAggrefUplevel - - * In the newly created sublink for an aggregate column used in - * the qualification, we must increment the varlevelsup in all the - * var nodes. - * - * NOTE: although this has the form of a walker, we cheat and modify the - * Var nodes in-place. The given expression tree should have been copied - * earlier to ensure that no unwanted side-effects occur! - */ -static bool -modifyAggrefUplevel(Node *node, void *context) -{ - if (node == NULL) - return false; - if (IsA(node, Var)) - { - Var *var = (Var *) node; - - var->varlevelsup++; - return false; - } - if (IsA(node, SubLink)) - { - /* - * Standard expression_tree_walker will not recurse into subselect, - * but here we must do so. - */ - SubLink *sub = (SubLink *) node; - - if (modifyAggrefUplevel((Node *) (sub->lefthand), context)) - return true; - if (modifyAggrefUplevel((Node *) (sub->subselect), context)) - return true; - return false; - } - if (IsA(node, Query)) - { - /* Reach here after recursing down into subselect above... */ - Query *qry = (Query *) node; - - if (modifyAggrefUplevel((Node *) (qry->targetList), context)) - return true; - if (modifyAggrefUplevel((Node *) (qry->qual), context)) - return true; - if (modifyAggrefUplevel((Node *) (qry->havingQual), context)) - return true; - return false; - } - return expression_tree_walker(node, modifyAggrefUplevel, - (void *) context); -} - - /* * modifyAggrefChangeVarnodes - * Change the var nodes in a sublink created for an aggregate column * used in the qualification to point to the correct local RTE. * + * XXX if we still need this after redoing querytree design, it should + * be combined with ChangeVarNodes, which is the same thing except for + * not having the option to adjust the vars' varlevelsup. + * * NOTE: although this has the form of a walker, we cheat and modify the * Var nodes in-place. The given expression tree should have been copied * earlier to ensure that no unwanted side-effects occur! @@ -547,18 +492,18 @@ modifyAggrefMakeSublink(Aggref *aggref, Query *parsetree) * Recursing would be a bad idea --- we'd likely produce an * infinite recursion. This whole technique is a crock, really... */ - if (checkQueryHasAggs(subquery->qual)) + if (checkExprHasAggs(subquery->qual)) elog(ERROR, "Cannot handle multiple aggregate functions in WHERE clause"); subquery->groupClause = NIL; subquery->havingQual = NULL; subquery->hasAggs = TRUE; - subquery->hasSubLinks = checkQueryHasSubLink(subquery->qual); + subquery->hasSubLinks = checkExprHasSubLink(subquery->qual); subquery->unionClause = NULL; /* Increment all varlevelsup fields in the new subquery */ - modifyAggrefUplevel((Node *) subquery, NULL); + IncrementVarSublevelsUp((Node *) subquery, 1, 0); - /* Replace references to the target table with correct varno. + /* Replace references to the target table with correct local varno. * Note +1 here to account for effects of previous line! */ modifyAggrefChangeVarnodes((Node *) subquery, target->varno, @@ -600,49 +545,6 @@ modifyAggrefQual(Node *node, Query *parsetree) } -/* - * checkQueryHasAggs - - * Queries marked hasAggs might not have them any longer after - * rewriting. Check it. - */ -static bool -checkQueryHasAggs(Node *node) -{ - return checkQueryHasAggs_walker(node, NULL); -} - -static bool -checkQueryHasAggs_walker(Node *node, void *context) -{ - if (node == NULL) - return false; - if (IsA(node, Aggref)) - return true; /* abort the tree traversal and return true */ - return expression_tree_walker(node, checkQueryHasAggs_walker, context); -} - -/* - * checkQueryHasSubLink - - * Queries marked hasSubLinks might not have them any longer after - * rewriting. Check it. - */ -static bool -checkQueryHasSubLink(Node *node) -{ - return checkQueryHasSubLink_walker(node, NULL); -} - -static bool -checkQueryHasSubLink_walker(Node *node, void *context) -{ - if (node == NULL) - return false; - if (IsA(node, SubLink)) - return true; /* abort the tree traversal and return true */ - return expression_tree_walker(node, checkQueryHasSubLink_walker, context); -} - - static Node * FindMatchingTLEntry(List *tlist, char *e_attname) { @@ -675,38 +577,6 @@ make_null(Oid type) } -/* - * apply_RIR_adjust_sublevel - - * Set the varlevelsup field of all Var nodes in the given expression tree - * to sublevels_up. We do NOT recurse into subselects. - * - * NOTE: although this has the form of a walker, we cheat and modify the - * Var nodes in-place. The given expression tree should have been copied - * earlier to ensure that no unwanted side-effects occur! - */ -static bool -apply_RIR_adjust_sublevel_walker(Node *node, int *sublevels_up) -{ - if (node == NULL) - return false; - if (IsA(node, Var)) - { - Var *var = (Var *) node; - - var->varlevelsup = *sublevels_up; - return false; - } - return expression_tree_walker(node, apply_RIR_adjust_sublevel_walker, - (void *) sublevels_up); -} - -static void -apply_RIR_adjust_sublevel(Node *node, int sublevels_up) -{ - apply_RIR_adjust_sublevel_walker(node, &sublevels_up); -} - - /* * apply_RIR_view * Replace Vars matching a given RT index with copies of TL expressions. @@ -749,9 +619,12 @@ apply_RIR_view_mutator(Node *node, return make_null(var->vartype); } + /* Make a copy of the tlist item to return */ expr = copyObject(expr); + /* Adjust varlevelsup if tlist item is from higher query level */ if (var->varlevelsup > 0) - apply_RIR_adjust_sublevel(expr, var->varlevelsup); + IncrementVarSublevelsUp(expr, var->varlevelsup, 0); + *(context->modified) = true; return (Node *) expr; } @@ -1279,8 +1152,11 @@ fireRules(Query *parsetree, else qual_product = (Query *) nth(0, *qual_products); + MemSet(&qual_info, 0, sizeof(qual_info)); qual_info.event = qual_product->commandType; + qual_info.current_varno = rt_index; qual_info.new_varno = length(qual_product->rtable) + 2; + qual_product = CopyAndAddQual(qual_product, actions, event_qual, @@ -1575,16 +1451,16 @@ BasicQueryRewrite(Query *parsetree) if (query->hasAggs) { query->hasAggs = - checkQueryHasAggs((Node *) (query->targetList)) || - checkQueryHasAggs((Node *) (query->havingQual)); - if (checkQueryHasAggs((Node *) (query->qual))) + checkExprHasAggs((Node *) (query->targetList)) || + checkExprHasAggs((Node *) (query->havingQual)); + if (checkExprHasAggs((Node *) (query->qual))) elog(ERROR, "BasicQueryRewrite: failed to remove aggs from qual"); } if (query->hasSubLinks) query->hasSubLinks = - checkQueryHasSubLink((Node *) (query->targetList)) || - checkQueryHasSubLink((Node *) (query->qual)) || - checkQueryHasSubLink((Node *) (query->havingQual)); + checkExprHasSubLink((Node *) (query->targetList)) || + checkExprHasSubLink((Node *) (query->qual)) || + checkExprHasSubLink((Node *) (query->havingQual)); results = lappend(results, query); } diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index 01f33ecf8f..e8224d80b3 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.44 2000/01/27 18:11:37 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.45 2000/03/16 03:23:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -31,6 +31,52 @@ #define MUTATE(newfield, oldfield, fieldtype, mutator, context) \ ( (newfield) = (fieldtype) mutator((Node *) (oldfield), (context)) ) +static bool checkExprHasAggs_walker(Node *node, void *context); +static bool checkExprHasSubLink_walker(Node *node, void *context); + + +/* + * checkExprHasAggs - + * Queries marked hasAggs might not have them any longer after + * rewriting. Check it. + */ +bool +checkExprHasAggs(Node *node) +{ + return checkExprHasAggs_walker(node, NULL); +} + +static bool +checkExprHasAggs_walker(Node *node, void *context) +{ + if (node == NULL) + return false; + if (IsA(node, Aggref)) + return true; /* abort the tree traversal and return true */ + return expression_tree_walker(node, checkExprHasAggs_walker, context); +} + +/* + * checkExprHasSubLink - + * Queries marked hasSubLinks might not have them any longer after + * rewriting. Check it. + */ +bool +checkExprHasSubLink(Node *node) +{ + return checkExprHasSubLink_walker(node, NULL); +} + +static bool +checkExprHasSubLink_walker(Node *node, void *context) +{ + if (node == NULL) + return false; + if (IsA(node, SubLink)) + return true; /* abort the tree traversal and return true */ + return expression_tree_walker(node, checkExprHasSubLink_walker, context); +} + /* * OffsetVarNodes - adjust Vars when appending one query's RT to another @@ -194,6 +240,89 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up) ChangeVarNodes_walker(node, &context); } +/* + * IncrementVarSublevelsUp - adjust Var nodes when pushing them down in tree + * + * Find all Var nodes in the given tree having varlevelsup >= min_sublevels_up, + * and add delta_sublevels_up to their varlevelsup value. This is needed when + * an expression that's correct for some nesting level is inserted into a + * subquery. Ordinarily the initial call has min_sublevels_up == 0 so that + * all Vars are affected. The point of min_sublevels_up is that we can + * increment it when we recurse into a sublink, so that local variables in + * that sublink are not affected, only outer references to vars that belong + * to the expression's original query level or parents thereof. + * + * NOTE: although this has the form of a walker, we cheat and modify the + * Var nodes in-place. The given expression tree should have been copied + * earlier to ensure that no unwanted side-effects occur! + */ + +typedef struct { + int delta_sublevels_up; + int min_sublevels_up; +} IncrementVarSublevelsUp_context; + +static bool +IncrementVarSublevelsUp_walker(Node *node, + IncrementVarSublevelsUp_context *context) +{ + if (node == NULL) + return false; + if (IsA(node, Var)) + { + Var *var = (Var *) node; + + if (var->varlevelsup >= context->min_sublevels_up) + var->varlevelsup += context->delta_sublevels_up; + return false; + } + if (IsA(node, SubLink)) + { + /* + * Standard expression_tree_walker will not recurse into subselect, + * but here we must do so. + */ + SubLink *sub = (SubLink *) node; + + if (IncrementVarSublevelsUp_walker((Node *) (sub->lefthand), + context)) + return true; + IncrementVarSublevelsUp((Node *) (sub->subselect), + context->delta_sublevels_up, + context->min_sublevels_up + 1); + return false; + } + if (IsA(node, Query)) + { + /* Reach here after recursing down into subselect above... */ + Query *qry = (Query *) node; + + if (IncrementVarSublevelsUp_walker((Node *) (qry->targetList), + context)) + return true; + if (IncrementVarSublevelsUp_walker((Node *) (qry->qual), + context)) + return true; + if (IncrementVarSublevelsUp_walker((Node *) (qry->havingQual), + context)) + return true; + return false; + } + return expression_tree_walker(node, IncrementVarSublevelsUp_walker, + (void *) context); +} + +void +IncrementVarSublevelsUp(Node *node, int delta_sublevels_up, + int min_sublevels_up) +{ + IncrementVarSublevelsUp_context context; + + context.delta_sublevels_up = delta_sublevels_up; + context.min_sublevels_up = min_sublevels_up; + IncrementVarSublevelsUp_walker(node, &context); +} + /* * Add the given qualifier condition to the query's WHERE clause */ @@ -214,6 +343,13 @@ AddQual(Query *parsetree, Node *qual) parsetree->qual = copy; else parsetree->qual = (Node *) make_andclause(makeList(old, copy, -1)); + + /* + * Make sure query is marked correctly if added qual has sublinks or + * aggregates (not sure it can ever have aggs, but sublinks definitely). + */ + parsetree->hasAggs |= checkExprHasAggs(copy); + parsetree->hasSubLinks |= checkExprHasSubLink(copy); } /* @@ -237,6 +373,13 @@ AddHavingQual(Query *parsetree, Node *havingQual) parsetree->havingQual = copy; else parsetree->havingQual = (Node *) make_andclause(makeList(old, copy, -1)); + + /* + * Make sure query is marked correctly if added qual has sublinks or + * aggregates (not sure it can ever have aggs, but sublinks definitely). + */ + parsetree->hasAggs |= checkExprHasAggs(copy); + parsetree->hasSubLinks |= checkExprHasSubLink(copy); } #ifdef NOT_USED @@ -428,13 +571,9 @@ ResolveNew_mutator(Node *node, ResolveNew_context *context) { /* Make a copy of the tlist item to return */ n = copyObject(n); - if (IsA(n, Var)) - { - ((Var *) n)->varlevelsup = this_varlevelsup; - } - /* XXX what to do if tlist item is NOT a var? - * Should we be using something like apply_RIR_adjust_sublevel? - */ + /* Adjust varlevelsup if tlist item is from higher query */ + if (this_varlevelsup > 0) + IncrementVarSublevelsUp(n, this_varlevelsup, 0); return n; } } @@ -552,24 +691,26 @@ HandleRIRAttributeRule_mutator(Node *node, } else { - NameData name_to_look_for; - - NameStr(name_to_look_for)[0] = '\0'; - namestrcpy(&name_to_look_for, - (char *) get_attname(getrelid(this_varno, - context->rtable), - this_varattno)); - if (NameStr(name_to_look_for)[0]) + char *name_to_look_for; + + name_to_look_for = get_attname(getrelid(this_varno, + context->rtable), + this_varattno); + if (name_to_look_for) { Node *n; *context->modified = TRUE; n = FindMatchingTLEntry(context->targetlist, - (char *) &name_to_look_for); + name_to_look_for); if (n == NULL) return make_null(var->vartype); - else - return copyObject(n); + /* Make a copy of the tlist item to return */ + n = copyObject(n); + /* Adjust varlevelsup if tlist item is from higher query */ + if (this_varlevelsup > 0) + IncrementVarSublevelsUp(n, this_varlevelsup, 0); + return n; } } } diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h index 35997e5878..33ea824cf2 100644 --- a/src/include/rewrite/rewriteManip.h +++ b/src/include/rewrite/rewriteManip.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: rewriteManip.h,v 1.19 2000/01/26 05:58:30 momjian Exp $ + * $Id: rewriteManip.h,v 1.20 2000/03/16 03:23:17 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -20,11 +20,16 @@ extern void OffsetVarNodes(Node *node, int offset, int sublevels_up); extern void ChangeVarNodes(Node *node, int old_varno, int new_varno, int sublevels_up); +extern void IncrementVarSublevelsUp(Node *node, int delta_sublevels_up, + int min_sublevels_up); extern void AddQual(Query *parsetree, Node *qual); extern void AddHavingQual(Query *parsetree, Node *havingQual); extern void AddNotQual(Query *parsetree, Node *qual); extern void AddGroupClause(Query *parsetree, List *group_by, List *tlist); +extern bool checkExprHasAggs(Node *node); +extern bool checkExprHasSubLink(Node *node); + extern void FixNew(RewriteInfo *info, Query *parsetree); extern void HandleRIRAttributeRule(Query *parsetree, List *rtable, -- 2.40.0