app/vmselect/promql: extract rmoeveGroupTags function for removing unneeded tags from MetricName according to the given modifierExpr

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]

app/vmselect/promql/exec_test.go

@@ -2208,6 +2208,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)`
@@ -3854,6 +3865,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)`)