Fix some (more) problems with subselects in rules. Rewriter failed to
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 16 Mar 2000 03:23:18 +0000 (03:23 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 16 Mar 2000 03:23:18 +0000 (03:23 +0000)
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
src/backend/rewrite/rewriteManip.c
src/include/rewrite/rewriteManip.h

index 0ba42f05942d65973a5d58b250bef09878cbc200..ec22f1e6dd0e821d46351c27ac411c3fa9dd40e2 100644 (file)
@@ -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);
    }
 
index 01f33ecf8f35dfb5f36a4c5d1f30180c5b665dcc..e8224d80b3a507928dc8f4aeeda525ba2a3dc2a6 100644 (file)
@@ -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 $
  *
  *-------------------------------------------------------------------------
  */
 #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;
                }
            }
        }
index 35997e5878a6451ccdec45874d21890ae162c2c1..33ea824cf2febe111a0e481f0b0311cae5e37830 100644 (file)
@@ -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 $
  *
  *-------------------------------------------------------------------------
  */
 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,