From ee4585db33311db076bf04b1812ee711ca66b428 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@gmail.com>
Date: Fri, 13 Sep 2019 21:40:46 +0300
Subject: [PATCH] app/vmselect/promql: properly handle subqueries like
 `aggr_func(rollup_func(metric[window:step]))`

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/184
---
 app/vmselect/promql/eval.go   | 12 +++++++-----
 app/vmselect/promql/parser.go |  4 ++++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/app/vmselect/promql/eval.go b/app/vmselect/promql/eval.go
index 50062e4b5e..44956bd7b2 100644
--- a/app/vmselect/promql/eval.go
+++ b/app/vmselect/promql/eval.go
@@ -296,10 +296,10 @@ func tryGetArgRollupFuncWithMetricExpr(ae *aggrFuncExpr) (*funcExpr, newRollupFu
 		return fe, nrf
 	}
 	if re, ok := e.(*rollupExpr); ok {
-		if me, ok := re.Expr.(*metricExpr); !ok || me.IsEmpty() {
+		if me, ok := re.Expr.(*metricExpr); !ok || me.IsEmpty() || re.ForSubquery() {
 			return nil, nil
 		}
-		// e = rollupExpr(metricExpr)
+		// e = metricExpr[d]
 		fe := &funcExpr{
 			Name: "default_rollup",
 			Args: []expr{re},
@@ -321,15 +321,17 @@ func tryGetArgRollupFuncWithMetricExpr(ae *aggrFuncExpr) (*funcExpr, newRollupFu
 		if me.IsEmpty() {
 			return nil, nil
 		}
+		// e = rollupFunc(metricExpr)
 		return &funcExpr{
 			Name: fe.Name,
 			Args: []expr{me},
 		}, nrf
 	}
 	if re, ok := arg.(*rollupExpr); ok {
-		if me, ok := re.Expr.(*metricExpr); !ok || me.IsEmpty() {
+		if me, ok := re.Expr.(*metricExpr); !ok || me.IsEmpty() || re.ForSubquery() {
 			return nil, nil
 		}
+		// e = rollupFunc(metricExpr[d])
 		return fe, nrf
 	}
 	return nil, nil
@@ -374,8 +376,8 @@ func getRollupExprArg(arg expr) *rollupExpr {
 			Expr: arg,
 		}
 	}
-	if len(re.Step) == 0 && !re.InheritStep {
-		// Return standard rollup if it doesn't set step.
+	if !re.ForSubquery() {
+		// Return standard rollup if it doesn't contain subquery.
 		return re
 	}
 	me, ok := re.Expr.(*metricExpr)
diff --git a/app/vmselect/promql/parser.go b/app/vmselect/promql/parser.go
index b2511727ee..dffd367729 100644
--- a/app/vmselect/promql/parser.go
+++ b/app/vmselect/promql/parser.go
@@ -1550,6 +1550,10 @@ type rollupExpr struct {
 	InheritStep bool
 }
 
+func (re *rollupExpr) ForSubquery() bool {
+	return len(re.Step) > 0 || re.InheritStep
+}
+
 func (re *rollupExpr) AppendString(dst []byte) []byte {
 	needParens := func() bool {
 		if _, ok := re.Expr.(*rollupExpr); ok {