From 20a3ddbbf93a87b195c8fe140f817f2784fe5957 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 25 Apr 2009 16:44:56 +0000
Subject: [PATCH] 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.
---
src/backend/optimizer/plan/subselect.c | 35 ++++++++++++++++++++++--
src/test/regress/expected/aggregates.out | 10 +++++++
src/test/regress/sql/aggregates.sql | 6 ++++
3 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 595ff1d46bb..6839e5d99ba 100644
--- 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
- * $PostgreSQL: pgsql/src/backend/optimizer/plan/subselect.c,v 1.148 2009/04/05 19:59:40 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/optimizer/plan/subselect.c,v 1.149 2009/04/25 16:44:56 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -437,13 +437,24 @@ build_subplan(PlannerInfo *root, Plan *plan, List *rtable,
if (pitem->abslevel == root->query_level)
{
- splan->parParam = lappend_int(splan->parParam, paramid);
+ 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.
*/
- splan->args = lappend(splan->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(root, arg, false);
+
+ splan->parParam = lappend_int(splan->parParam, paramid);
+ splan->args = lappend(splan->args, arg);
}
}
bms_free(tmpset);
@@ -1531,6 +1542,12 @@ convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect,
* 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 build_subplan).
*/
Node *
SS_replace_correlation_vars(PlannerInfo *root, Node *expr)
@@ -1607,6 +1624,18 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context)
context->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 build_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 find
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 46f6f18ed05..a48b45007f3 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -295,6 +295,16 @@ having exists (select 1 from onek b
ERROR: aggregates not allowed in WHERE clause
LINE 4: 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;
+ ?column?
+----------
+ 9999
+(1 row)
+
--
-- test for bitwise integer aggregates
--
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index afa997e7ea7..39b0187a1e8 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -80,6 +80,12 @@ 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;
+
--
-- test for bitwise integer aggregates
--
--
GitLab