Fix the handling of sub-SELECTs appearing in the arguments of an outer-level

aggregate function.  By definition, such a sub-SELECT cannot reference any
variables of query levels between itself and the aggregate's semantic level
(else the aggregate would've been assigned to that lower level instead).
So the correct, most efficient implementation is to treat the sub-SELECT as
being a sub-select of that outer query level, not the level the aggregate
syntactically appears in.  Not doing so also confuses the heck out of our
parameter-passing logic, as illustrated in bug report from Daniel Grace.

Fortunately, we were already copying the whole Aggref expression up to the
outer query level, so all that's needed is to delay SS_process_sublinks
processing of the sub-SELECT until control returns to the outer level.

This has been broken since we introduced spec-compliant treatment of
outer aggregates in 7.4; so patch all the way back.

diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index be53ba80be70fd588e23b1df84cc7bbcb94aae3e..c81e7e133b00ae47b3a00180a45ca29487f588a9 100644 (file)
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v 1.83.2.2 2004/05/11 13:15:23 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v 1.83.2.3 2009/04/25 16:45:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -474,13 +474,23 @@ make_subplan(SubLink *slink, List *lefthand, bool isTopQual)
                foreach(lst, node->parParam)
                {
                        PlannerParamItem *pitem = nth(lfirsti(lst), PlannerParamList);
+                       Node   *arg;
 
                        /*
                         * The Var or Aggref has already been adjusted to have the
                         * correct varlevelsup or agglevelsup.  We probably don't even
                         * need to copy it again, but be safe.
                         */
-                       args = lappend(args, copyObject(pitem->item));
+                       arg = copyObject(pitem->item);
+
+                       /*
+                        * If it's an Aggref, its arguments might contain SubLinks,
+                        * which have not yet been processed.  Do that now.
+                        */
+                       if (IsA(arg, Aggref))
+                               arg = SS_process_sublinks(arg, false);
+
+                       args = lappend(args, arg);
                }
                node->args = args;
 
@@ -771,6 +781,12 @@ convert_IN_to_join(Query *parse, SubLink *sublink)
  * so after expanding its sublinks to subplans.  And we don't want any steps
  * in between, else those steps would never get applied to the aggregate
  * argument expressions, either in the parent or the child level.
+ *
+ * Another fairly tricky thing going on here is the handling of SubLinks in
+ * the arguments of uplevel aggregates.  Those are not touched inside the
+ * intermediate query level, either.  Instead, SS_process_sublinks recurses
+ * on them after copying the Aggref expression into the parent plan level
+ * (this is actually taken care of in make_subplan).
  */
 Node *
 SS_replace_correlation_vars(Node *expr)
@@ -839,6 +855,18 @@ process_sublinks_mutator(Node *node, bool *isTopQual)
                return make_subplan(sublink, lefthand, *isTopQual);
        }
 
+       /*
+        * Don't recurse into the arguments of an outer aggregate here.
+        * Any SubLinks in the arguments have to be dealt with at the outer
+        * query level; they'll be handled when make_subplan collects the
+        * Aggref into the arguments to be passed down to the current subplan.
+        */
+       if (IsA(node, Aggref))
+       {
+               if (((Aggref *) node)->agglevelsup > 0)
+                       return node;
+       }
+
        /*
         * We should never see a SubPlan expression in the input (since this
         * is the very routine that creates 'em to begin with).  We shouldn't

diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 62f31d395ed3dcc1c9aad7043705522d6f1c2296..3b7618d61d54d940cd11b98444c14033bb6300b5 100644 (file)
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -157,3 +157,13 @@ group by ten
 having exists (select 1 from onek b
                where sum(distinct a.four + b.four) = b.four);
 ERROR:  aggregates not allowed in WHERE clause
+-- Test handling of sublinks within outer-level aggregates.
+-- Per bug report from Daniel Grace.
+select
+  (select max((select i.unique2 from tenk1 i where i.unique1 = o.unique1)))
+from tenk1 o;
+ ?column? 
+----------
+     9999
+(1 row)
+

diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 38335bcf083cbd057603d9ee7f0e6ae81bd7b8c8..13f447a158e298c274b96da6d49446666cb7c228 100644 (file)
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -62,3 +62,9 @@ select ten, sum(distinct four) from onek a
 group by ten
 having exists (select 1 from onek b
                where sum(distinct a.four + b.four) = b.four);
+
+-- Test handling of sublinks within outer-level aggregates.
+-- Per bug report from Daniel Grace.
+select
+  (select max((select i.unique2 from tenk1 i where i.unique1 = o.unique1)))
+from tenk1 o;