app/vmselect: properly handle PromQL queries like scalar1 < metric < scalar2 like Prometheus does

This fixes some cases from https://promlabs.com/promql-compliance-test-results-victoriametrics/

app/vmselect/promql/exec.go

@@ -133,10 +133,50 @@ func removeNaNs(tss []*timeseries) []*timeseries {
 	return rvs
 }
 
+func adjustCmpOps(e metricsql.Expr) metricsql.Expr {
+	metricsql.VisitAll(e, func(expr metricsql.Expr) {
+		be, ok := expr.(*metricsql.BinaryOpExpr)
+		if !ok {
+			return
+		}
+		if !metricsql.IsBinaryOpCmp(be.Op) {
+			return
+		}
+		if _, ok := be.Left.(*metricsql.NumberExpr); !ok {
+			return
+		}
+		// Convert 'num cmpOp query' expression to `query reverseCmpOp num` expression
+		// like Prometheus does. For isntance, `0.5 < foo` must be converted to `foo > 0.5`
+		// in order to return valid values for `foo` that are bigger than 0.5.
+		be.Right, be.Left = be.Left, be.Right
+		be.Op = getReverseCmpOp(be.Op)
+	})
+	return e
+}
+
+func getReverseCmpOp(op string) string {
+	switch op {
+	case ">":
+		return "<"
+	case "<":
+		return ">"
+	case ">=":
+		return "<="
+	case "<=":
+		return ">="
+	default:
+		// there is no need in changing `==` and `!=`.
+		return op
+	}
+}
+
 func parsePromQLWithCache(q string) (metricsql.Expr, error) {
 	pcv := parseCacheV.Get(q)
 	if pcv == nil {
 		e, err := metricsql.Parse(q)
+		if err == nil {
+			e = adjustCmpOps(e)
+		}
 		pcv = &parseCacheValue{
 			e:   e,
 			err: err,

app/vmselect/promql/exec_test.go

@@ -1713,6 +1713,51 @@ func TestExecSuccess(t *testing.T) {
 		resultExpected := []netstorage.Result{r1, r2}
 		f(q, resultExpected)
 	})
+	t.Run(`scalar < time()`, func(t *testing.T) {
+		t.Parallel()
+		q := `123 < time()`
+		r := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{1000, 1200, 1400, 1600, 1800, 2000},
+			Timestamps: timestampsExpected,
+		}
+		resultExpected := []netstorage.Result{r}
+		f(q, resultExpected)
+	})
+	t.Run(`time() > scalar`, func(t *testing.T) {
+		t.Parallel()
+		q := `time() > 123`
+		r := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{1000, 1200, 1400, 1600, 1800, 2000},
+			Timestamps: timestampsExpected,
+		}
+		resultExpected := []netstorage.Result{r}
+		f(q, resultExpected)
+	})
+	t.Run(`scalar > time()`, func(t *testing.T) {
+		t.Parallel()
+		q := `123 > time()`
+		resultExpected := []netstorage.Result{}
+		f(q, resultExpected)
+	})
+	t.Run(`time() < scalar`, func(t *testing.T) {
+		t.Parallel()
+		q := `time() < 123`
+		resultExpected := []netstorage.Result{}
+		f(q, resultExpected)
+	})
+	t.Run(`scalar1 < time() < scalar2`, func(t *testing.T) {
+		t.Parallel()
+		q := `1300 < time() < 1700`
+		r := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{nan, nan, 1400, 1600, nan, nan},
+			Timestamps: timestampsExpected,
+		}
+		resultExpected := []netstorage.Result{r}
+		f(q, resultExpected)
+	})
 	t.Run(`a cmp scalar (leave MetricGroup)`, func(t *testing.T) {
 		t.Parallel()
 		q := `sort_desc((

go.mod

@@ -8,7 +8,7 @@ require (
 	// like https://github.com/valyala/fasthttp/commit/996610f021ff45fdc98c2ce7884d5fa4e7f9199b
 	github.com/VictoriaMetrics/fasthttp v1.0.4
 	github.com/VictoriaMetrics/metrics v1.12.2
-	github.com/VictoriaMetrics/metricsql v0.3.0
+	github.com/VictoriaMetrics/metricsql v0.4.0
 	github.com/aws/aws-sdk-go v1.33.19
 	github.com/cespare/xxhash/v2 v2.1.1
 	github.com/golang/snappy v0.0.1

go.sum

@@ -51,8 +51,8 @@ github.com/VictoriaMetrics/fasthttp v1.0.4 h1:Aw6UqmPc0v5PunYOpiiyf4hk5B9PTMswdT
 github.com/VictoriaMetrics/fasthttp v1.0.4/go.mod h1:m5wCmg1dJN6s1B/lp8/dcKrnnM2l3DWB2eAGZGCTK3g=
 github.com/VictoriaMetrics/metrics v1.12.2 h1:SG8iAmqavDNuh7GIdHPoGHUhDL23KeKfvSZSozucNeA=
 github.com/VictoriaMetrics/metrics v1.12.2/go.mod h1:Z1tSfPfngDn12bTfZSCqArT3OPY3u88J12hSoOhuiRE=
-github.com/VictoriaMetrics/metricsql v0.3.0 h1:610HmpZixn1KiQhV+/BNK/OAHE5M+8Fiy8LtI/xEguo=
-github.com/VictoriaMetrics/metricsql v0.3.0/go.mod h1:ylO7YITho/Iw6P71oEaGyHbO94bGoGtzWfLGqFhMIg8=
+github.com/VictoriaMetrics/metricsql v0.4.0 h1:1CMGiUzOgR83Y+hVS83/DaHYLaR6PFw1XWcvQnHn2V8=
+github.com/VictoriaMetrics/metricsql v0.4.0/go.mod h1:ylO7YITho/Iw6P71oEaGyHbO94bGoGtzWfLGqFhMIg8=
 github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah4HI848JfFxHt+iPb26b4zyfspmqY0/8=
 github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM=
 github.com/andybalholm/brotli v1.0.0/go.mod h1:loMXtMfwqflxFJPmdbJO0a3KNoPuLBgiu3qAvBg8x/Y=

vendor/github.com/VictoriaMetrics/metricsql/utils.go

@@ -10,3 +10,29 @@ func ExpandWithExprs(q string) (string, error) {
 	buf := e.AppendString(nil)
 	return string(buf), nil
 }
+
+// VisitAll recursively calls f for all the Expr children in e.
+//
+// It visits leaf children at first and then visits parent nodes.
+// It is safe modifying expr in f.
+func VisitAll(e Expr, f func(expr Expr)) {
+	switch expr := e.(type) {
+	case *BinaryOpExpr:
+		VisitAll(expr.Left, f)
+		VisitAll(expr.Right, f)
+		VisitAll(&expr.GroupModifier, f)
+		VisitAll(&expr.JoinModifier, f)
+	case *FuncExpr:
+		for _, arg := range expr.Args {
+			VisitAll(arg, f)
+		}
+	case *AggrFuncExpr:
+		for _, arg := range expr.Args {
+			VisitAll(arg, f)
+		}
+		VisitAll(&expr.Modifier, f)
+	case *RollupExpr:
+		VisitAll(expr.Expr, f)
+	}
+	f(e)
+}

vendor/modules.txt

@@ -16,7 +16,7 @@ github.com/VictoriaMetrics/fasthttp/fasthttputil
 github.com/VictoriaMetrics/fasthttp/stackless
 # github.com/VictoriaMetrics/metrics v1.12.2
 github.com/VictoriaMetrics/metrics
-# github.com/VictoriaMetrics/metricsql v0.3.0
+# github.com/VictoriaMetrics/metricsql v0.4.0
 github.com/VictoriaMetrics/metricsql
 github.com/VictoriaMetrics/metricsql/binaryop
 # github.com/aws/aws-sdk-go v1.33.19