Commit 48b6035f authored by Tom Lane's avatar Tom Lane

Fix assorted missing logic for GroupingFunc nodes.

The planner needs to treat GroupingFunc like Aggref for many purposes,
in particular with respect to processing of the argument expressions,
which are not to be evaluated at runtime.  A few places hadn't gotten
that memo, notably including subselect.c's processing of outer-level
aggregates.  This resulted in assertion failures or wrong plans for
cases in which a GROUPING() construct references an outer aggregation
level.

Also fix missing special cases for GroupingFunc in cost_qual_eval
(resulting in wrong cost estimates for GROUPING(), although it's
not clear that that would affect plan shapes in practice) and in
ruleutils.c (resulting in excess parentheses in pretty-print mode).

Per bug #17088 from Yaoguang Chen.  Back-patch to all supported
branches.

Richard Guo, Tom Lane

Discussion: https://postgr.es/m/17088-e33882b387de7f5c@postgresql.org
parent 05ccf974
...@@ -736,6 +736,8 @@ expression_returns_set_walker(Node *node, void *context) ...@@ -736,6 +736,8 @@ expression_returns_set_walker(Node *node, void *context)
/* Avoid recursion for some cases that parser checks not to return a set */ /* Avoid recursion for some cases that parser checks not to return a set */
if (IsA(node, Aggref)) if (IsA(node, Aggref))
return false; return false;
if (IsA(node, GroupingFunc))
return false;
if (IsA(node, WindowFunc)) if (IsA(node, WindowFunc))
return false; return false;
......
...@@ -4492,6 +4492,12 @@ cost_qual_eval_walker(Node *node, cost_qual_eval_context *context) ...@@ -4492,6 +4492,12 @@ cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
*/ */
return false; /* don't recurse into children */ return false; /* don't recurse into children */
} }
else if (IsA(node, GroupingFunc))
{
/* Treat this as having cost 1 */
context->total.per_tuple += cpu_operator_cost;
return false; /* don't recurse into children */
}
else if (IsA(node, CoerceViaIO)) else if (IsA(node, CoerceViaIO))
{ {
CoerceViaIO *iocoerce = (CoerceViaIO *) node; CoerceViaIO *iocoerce = (CoerceViaIO *) node;
......
...@@ -356,15 +356,17 @@ build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot, ...@@ -356,15 +356,17 @@ build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot,
Node *arg = pitem->item; Node *arg = pitem->item;
/* /*
* The Var, PlaceHolderVar, or Aggref has already been adjusted to * The Var, PlaceHolderVar, Aggref or GroupingFunc has already been
* have the correct varlevelsup, phlevelsup, or agglevelsup. * adjusted to have the correct varlevelsup, phlevelsup, or
* agglevelsup.
* *
* If it's a PlaceHolderVar or Aggref, its arguments might contain * If it's a PlaceHolderVar, Aggref or GroupingFunc, its arguments
* SubLinks, which have not yet been processed (see the comments for * might contain SubLinks, which have not yet been processed (see the
* SS_replace_correlation_vars). Do that now. * comments for SS_replace_correlation_vars). Do that now.
*/ */
if (IsA(arg, PlaceHolderVar) || if (IsA(arg, PlaceHolderVar) ||
IsA(arg, Aggref)) IsA(arg, Aggref) ||
IsA(arg, GroupingFunc))
arg = SS_process_sublinks(root, arg, false); arg = SS_process_sublinks(root, arg, false);
splan->parParam = lappend_int(splan->parParam, pitem->paramId); splan->parParam = lappend_int(splan->parParam, pitem->paramId);
...@@ -1957,10 +1959,11 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context) ...@@ -1957,10 +1959,11 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context)
} }
/* /*
* Don't recurse into the arguments of an outer PHV or aggregate here. Any * Don't recurse into the arguments of an outer PHV, Aggref or
* SubLinks in the arguments have to be dealt with at the outer query * GroupingFunc here. Any SubLinks in the arguments have to be dealt with
* level; they'll be handled when build_subplan collects the PHV or Aggref * at the outer query level; they'll be handled when build_subplan
* into the arguments to be passed down to the current subplan. * collects the PHV, Aggref or GroupingFunc into the arguments to be
* passed down to the current subplan.
*/ */
if (IsA(node, PlaceHolderVar)) if (IsA(node, PlaceHolderVar))
{ {
...@@ -1972,6 +1975,11 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context) ...@@ -1972,6 +1975,11 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context)
if (((Aggref *) node)->agglevelsup > 0) if (((Aggref *) node)->agglevelsup > 0)
return node; return node;
} }
else if (IsA(node, GroupingFunc))
{
if (((GroupingFunc *) node)->agglevelsup > 0)
return node;
}
/* /*
* We should never see a SubPlan expression in the input (since this is * We should never see a SubPlan expression in the input (since this is
...@@ -2084,7 +2092,7 @@ SS_identify_outer_params(PlannerInfo *root) ...@@ -2084,7 +2092,7 @@ SS_identify_outer_params(PlannerInfo *root)
outer_params = NULL; outer_params = NULL;
for (proot = root->parent_root; proot != NULL; proot = proot->parent_root) for (proot = root->parent_root; proot != NULL; proot = proot->parent_root)
{ {
/* Include ordinary Var/PHV/Aggref params */ /* Include ordinary Var/PHV/Aggref/GroupingFunc params */
foreach(l, proot->plan_params) foreach(l, proot->plan_params)
{ {
PlannerParamItem *pitem = (PlannerParamItem *) lfirst(l); PlannerParamItem *pitem = (PlannerParamItem *) lfirst(l);
......
...@@ -7895,12 +7895,13 @@ get_parameter(Param *param, deparse_context *context) ...@@ -7895,12 +7895,13 @@ get_parameter(Param *param, deparse_context *context)
context->varprefix = true; context->varprefix = true;
/* /*
* A Param's expansion is typically a Var, Aggref, or upper-level * A Param's expansion is typically a Var, Aggref, GroupingFunc, or
* Param, which wouldn't need extra parentheses. Otherwise, insert * upper-level Param, which wouldn't need extra parentheses.
* parens to ensure the expression looks atomic. * Otherwise, insert parens to ensure the expression looks atomic.
*/ */
need_paren = !(IsA(expr, Var) || need_paren = !(IsA(expr, Var) ||
IsA(expr, Aggref) || IsA(expr, Aggref) ||
IsA(expr, GroupingFunc) ||
IsA(expr, Param)); IsA(expr, Param));
if (need_paren) if (need_paren)
appendStringInfoChar(context->buf, '('); appendStringInfoChar(context->buf, '(');
...@@ -8028,6 +8029,7 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) ...@@ -8028,6 +8029,7 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags)
case T_NextValueExpr: case T_NextValueExpr:
case T_NullIfExpr: case T_NullIfExpr:
case T_Aggref: case T_Aggref:
case T_GroupingFunc:
case T_WindowFunc: case T_WindowFunc:
case T_FuncExpr: case T_FuncExpr:
/* function-like: name(..) or name[..] */ /* function-like: name(..) or name[..] */
...@@ -8144,6 +8146,7 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) ...@@ -8144,6 +8146,7 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags)
case T_XmlExpr: /* own parentheses */ case T_XmlExpr: /* own parentheses */
case T_NullIfExpr: /* other separators */ case T_NullIfExpr: /* other separators */
case T_Aggref: /* own parentheses */ case T_Aggref: /* own parentheses */
case T_GroupingFunc: /* own parentheses */
case T_WindowFunc: /* own parentheses */ case T_WindowFunc: /* own parentheses */
case T_CaseExpr: /* other separators */ case T_CaseExpr: /* other separators */
return true; return true;
...@@ -8194,6 +8197,7 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags) ...@@ -8194,6 +8197,7 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags)
case T_XmlExpr: /* own parentheses */ case T_XmlExpr: /* own parentheses */
case T_NullIfExpr: /* other separators */ case T_NullIfExpr: /* other separators */
case T_Aggref: /* own parentheses */ case T_Aggref: /* own parentheses */
case T_GroupingFunc: /* own parentheses */
case T_WindowFunc: /* own parentheses */ case T_WindowFunc: /* own parentheses */
case T_CaseExpr: /* other separators */ case T_CaseExpr: /* other separators */
return true; return true;
......
...@@ -2040,4 +2040,49 @@ order by a, b, c; ...@@ -2040,4 +2040,49 @@ order by a, b, c;
| | | |
(11 rows) (11 rows)
-- test handling of outer GroupingFunc within subqueries
explain (costs off)
select (select grouping(v1)) from (values ((select 1))) v(v1) group by cube(v1);
QUERY PLAN
---------------------------
MixedAggregate
Hash Key: $2
Group Key: ()
InitPlan 1 (returns $1)
-> Result
InitPlan 3 (returns $2)
-> Result
-> Result
SubPlan 2
-> Result
(10 rows)
select (select grouping(v1)) from (values ((select 1))) v(v1) group by cube(v1);
grouping
----------
1
0
(2 rows)
explain (costs off)
select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1;
QUERY PLAN
---------------------------
GroupAggregate
Group Key: $2
InitPlan 1 (returns $1)
-> Result
InitPlan 3 (returns $2)
-> Result
-> Result
SubPlan 2
-> Result
(9 rows)
select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1;
grouping
----------
0
(1 row)
-- end -- end
...@@ -555,4 +555,13 @@ from (values (1, 2, 3), (4, null, 6), (7, 8, 9)) as t (a, b, c) ...@@ -555,4 +555,13 @@ from (values (1, 2, 3), (4, null, 6), (7, 8, 9)) as t (a, b, c)
group by rollup(a, b), rollup(a, c) group by rollup(a, b), rollup(a, c)
order by a, b, c; order by a, b, c;
-- test handling of outer GroupingFunc within subqueries
explain (costs off)
select (select grouping(v1)) from (values ((select 1))) v(v1) group by cube(v1);
select (select grouping(v1)) from (values ((select 1))) v(v1) group by cube(v1);
explain (costs off)
select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1;
select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1;
-- end -- end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment