It is possible for ResolveNew to be used to insert a sublink into a

subquery that didn't previously have one.  We have traditionally made
the caller of ResolveNew responsible for updating the hasSubLinks flag
of the outermost query, but this fails to account for hasSubLinks in
subqueries.  Fix ResolveNew to handle this.  We might later want to
change the calling convention of ResolveNew so that it can fix the
outer query too, simplifying callers.  But I went with the localized
fix for now.  Per bug report from J Smith, 20-Oct-03.

diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index ea48d2f07a39a472bcfbba796f9f3c0d9bde65f3..17c13d68585a4fd3245f2c9e46231b6d4eb00c19 100644 (file)
--- 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.79 2003/09/25 06:58:01 petere Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.80 2003/10/20 20:01:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -99,7 +99,7 @@ bool
 checkExprHasSubLink(Node *node)
 {
        /*
-        * If a Query is passed, examine it --- but we will not recurse into
+        * If a Query is passed, examine it --- but we need not recurse into
         * sub-Queries.
         */
        return query_or_expression_tree_walker(node,
@@ -755,10 +755,12 @@ AddQual(Query *parsetree, Node *qual)
        /*
         * Make sure query is marked correctly if added qual has sublinks or
         * aggregates (not sure it can ever have aggs, but sublinks
-        * definitely).
+        * definitely).  Need not search qual when query is already marked.
         */
-       parsetree->hasAggs |= checkExprHasAggs(copy);
-       parsetree->hasSubLinks |= checkExprHasSubLink(copy);
+       if (!parsetree->hasAggs)
+               parsetree->hasAggs = checkExprHasAggs(copy);
+       if (!parsetree->hasSubLinks)
+               parsetree->hasSubLinks = checkExprHasSubLink(copy);
 }
 
 /*
@@ -809,10 +811,12 @@ AddHavingQual(Query *parsetree, Node *havingQual)
        /*
         * Make sure query is marked correctly if added qual has sublinks or
         * aggregates (not sure it can ever have aggs, but sublinks
-        * definitely).
+        * definitely).  Need not search qual when query is already marked.
         */
-       parsetree->hasAggs |= checkExprHasAggs(copy);
-       parsetree->hasSubLinks |= checkExprHasSubLink(copy);
+       if (!parsetree->hasAggs)
+               parsetree->hasAggs = checkExprHasAggs(copy);
+       if (!parsetree->hasSubLinks)
+               parsetree->hasSubLinks = checkExprHasSubLink(copy);
 }
 
 
@@ -845,6 +849,12 @@ AddInvertedQual(Query *parsetree, Node *qual)
  * entry with matching resno from targetlist, if there is one.
  * If not, we either change the unmatched Var's varno to update_varno
  * (when event == CMD_UPDATE) or replace it with a constant NULL.
+ *
+ * Note: the business with inserted_sublink is needed to update hasSubLinks
+ * in subqueries when the replacement adds a subquery inside a subquery.
+ * Messy, isn't it?  We do not need to do similar pushups for hasAggs,
+ * because it isn't possible for this transformation to insert a level-zero
+ * aggregate reference into a subquery --- it could only insert outer aggs.
  */
 
 typedef struct
@@ -854,6 +864,7 @@ typedef struct
        List       *targetlist;
        int                     event;
        int                     update_varno;
+       bool            inserted_sublink;
 } ResolveNew_context;
 
 static Node *
@@ -904,6 +915,9 @@ ResolveNew_mutator(Node *node, ResolveNew_context *context)
                                /* Adjust varlevelsup if tlist item is from higher query */
                                if (this_varlevelsup > 0)
                                        IncrementVarSublevelsUp(n, this_varlevelsup, 0);
+                               /* Report it if we are adding a sublink to query */
+                               if (!context->inserted_sublink)
+                                       context->inserted_sublink = checkExprHasSubLink(n);
                                return n;
                        }
                }
@@ -914,12 +928,17 @@ ResolveNew_mutator(Node *node, ResolveNew_context *context)
        {
                /* Recurse into RTE subquery or not-yet-planned sublink subquery */
                Query      *newnode;
+               bool            save_inserted_sublink;
 
                context->sublevels_up++;
+               save_inserted_sublink = context->inserted_sublink;
+               context->inserted_sublink = false;
                newnode = query_tree_mutator((Query *) node,
                                                                         ResolveNew_mutator,
                                                                         (void *) context,
                                                                         0);
+               newnode->hasSubLinks |= context->inserted_sublink;
+               context->inserted_sublink = save_inserted_sublink;
                context->sublevels_up--;
                return (Node *) newnode;
        }
@@ -938,6 +957,7 @@ ResolveNew(Node *node, int target_varno, int sublevels_up,
        context.targetlist = targetlist;
        context.event = event;
        context.update_varno = update_varno;
+       context.inserted_sublink = false;
 
        /*
         * Must be prepared to start with a Query or a bare expression tree;