From b3bb18d67457abad543b4a4e6f6ce349c6af2ebe Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Mon, 27 Feb 2023 11:47:26 -0800
Subject: [PATCH] app/vmselect/promql: fix panic when calculating
 `aggr_func(rollup*())`

The panic has been introduced in dac21d874b555a80166561376059961d861cbc76
---
 app/vmselect/promql/rollup.go | 15 ++++++++++++++-
 docs/CHANGELOG.md             |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/app/vmselect/promql/rollup.go b/app/vmselect/promql/rollup.go
index 1765316a4..f585440e5 100644
--- a/app/vmselect/promql/rollup.go
+++ b/app/vmselect/promql/rollup.go
@@ -282,10 +282,23 @@ func getRollupAggrFuncNames(expr metricsql.Expr) ([]string, error) {
 	return aggrFuncNames, nil
 }
 
+// getRollupTag returns the possible second arg from the expr.
+//
+// The expr can have the following forms:
+// - rollup_func(q, tag)
+// - aggr_func(rollup_func(q, tag)) - this form is used during incremental aggregate calculations
 func getRollupTag(expr metricsql.Expr) (string, error) {
+	af, ok := expr.(*metricsql.AggrFuncExpr)
+	if ok {
+		// extract rollup_func() from aggr_func(rollup_func(q, tag))
+		if len(af.Args) != 1 {
+			logger.Panicf("BUG: unexpected number of args to %s; got %d; want 1", af.AppendString(nil), len(af.Args))
+		}
+		expr = af.Args[0]
+	}
 	fe, ok := expr.(*metricsql.FuncExpr)
 	if !ok {
-		logger.Panicf("BUG: unexpected expression; want metricsql.FuncExpr; got %T; value: %s", expr, expr.AppendString(nil))
+		logger.Panicf("BUG: unexpected expression; want *metricsql.FuncExpr; got %T; value: %s", expr, expr.AppendString(nil))
 	}
 	if len(fe.Args) < 2 {
 		return "", nil
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index a67a22940..3c57f6d12 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -15,6 +15,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 
 ## tip
 
+* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): fix panic when executing the query `aggr_func(rollup*(some_value))`. The panic has been introduced in [v1.88.0](https://docs.victoriametrics.com/CHANGELOG.html#v1880).
 * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): use the provided `-remoteWrite.*` auth options when determining whether the remote storage supports [VictoriaMetrics remote write protocol](https://docs.victoriametrics.com/vmagent.html#victoriametrics-remote-write-protocol). Previously the auth options were ignored. This was preventing from automatic switch to VictoriaMetrics remote write protocol.
 * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): skip measurements with no fields when migrating data from influxdb. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3837).