diff --git a/app/vmselect/promql/aggr.go b/app/vmselect/promql/aggr.go index 2732fb029..51eaec99a 100644 --- a/app/vmselect/promql/aggr.go +++ b/app/vmselect/promql/aggr.go @@ -6,6 +6,9 @@ import ( "sort" "strconv" "strings" + + "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" + "github.com/VictoriaMetrics/VictoriaMetrics/lib/storage" ) var aggrFuncs = map[string]aggrFunc{ @@ -67,33 +70,26 @@ func newAggrFunc(afe func(tss []*timeseries) []*timeseries) aggrFunc { } } +func removeGroupTags(metricName *storage.MetricName, modifier *modifierExpr) { + groupOp := strings.ToLower(modifier.Op) + switch groupOp { + case "", "by": + metricName.RemoveTagsOn(modifier.Args) + case "without": + metricName.RemoveTagsIgnoring(modifier.Args) + default: + logger.Panicf("BUG: unknown group modifier: %q", groupOp) + } +} + func aggrFuncExt(afe func(tss []*timeseries) []*timeseries, argOrig []*timeseries, modifier *modifierExpr, keepOriginal bool) ([]*timeseries, error) { arg := copyTimeseriesMetricNames(argOrig) - // Filter out superflouos tags. - var groupTags []string - groupOp := "by" - if modifier.Op != "" { - groupTags = modifier.Args - groupOp = strings.ToLower(modifier.Op) - } - switch groupOp { - case "by": - for _, ts := range arg { - ts.MetricName.RemoveTagsOn(groupTags) - } - case "without": - for _, ts := range arg { - ts.MetricName.RemoveTagsIgnoring(groupTags) - } - default: - return nil, fmt.Errorf(`unknown modifier: %q`, groupOp) - } - // Perform grouping. m := make(map[string][]*timeseries) bb := bbPool.Get() for i, ts := range arg { + removeGroupTags(&ts.MetricName, modifier) bb.B = marshalMetricNameSorted(bb.B[:0], &ts.MetricName) if keepOriginal { ts = argOrig[i] diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index e442a5cb0..d81f8fc84 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -2218,6 +2218,17 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r} f(q, resultExpected) }) + t.Run(`avg(scalar) wiTHout (xx, yy)`, func(t *testing.T) { + t.Parallel() + q := `avg wiTHout (xx, yy) (123)` + r := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{123, 123, 123, 123, 123, 123}, + Timestamps: timestampsExpected, + } + resultExpected := []netstorage.Result{r} + f(q, resultExpected) + }) t.Run(`sum(time)`, func(t *testing.T) { t.Parallel() q := `sum(time()/100)` @@ -3868,6 +3879,13 @@ func TestExecError(t *testing.T) { label_set(time()+200, "__name__", "bar", "a", "x"), ) + 10`) + // Invalid aggregates + f(`sum(1, 2)`) + f(`sum(1) foo (bar)`) + f(`sum foo () (bar)`) + f(`sum(foo) by (1)`) + f(`count(foo) without ("bar")`) + // With expressions f(`ttf()`) f(`ttf(1, 2)`)