From 262cf8175758e79202da3302b788cca64769e78f Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@gmail.com>
Date: Sat, 19 Dec 2020 01:23:44 +0200
Subject: [PATCH] app/vmselect: properly parse negative combined offsets such
 as `-1h2m3s`

Previously such offsets were parsed as `-1h + 2m + 3s`. Now they are parsed as `-(1h + 2m + 3s)`.
---
 app/vmselect/promql/exec_test.go                | 17 ++++++++++++++---
 docs/CHANGELOG.md                               |  1 +
 go.mod                                          |  2 +-
 go.sum                                          |  4 ++--
 .../VictoriaMetrics/metricsql/lexer.go          | 15 +++++++++++++--
 vendor/modules.txt                              |  2 +-
 6 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go
index 7b5228e812..679c9cc5dc 100644
--- a/app/vmselect/promql/exec_test.go
+++ b/app/vmselect/promql/exec_test.go
@@ -198,12 +198,23 @@ func TestExecSuccess(t *testing.T) {
 		resultExpected := []netstorage.Result{r}
 		f(q, resultExpected)
 	})
-	t.Run("time() offset 1m40s0ms", func(t *testing.T) {
+	t.Run("time() offset 1h40s0ms", func(t *testing.T) {
 		t.Parallel()
-		q := `time() offset 100s`
+		q := `time() offset 1h40s0ms`
 		r := netstorage.Result{
 			MetricName: metricNameExpected,
-			Values:     []float64{800, 1000, 1200, 1400, 1600, 1800},
+			Values:     []float64{-2800, -2600, -2400, -2200, -2000, -1800},
+			Timestamps: timestampsExpected,
+		}
+		resultExpected := []netstorage.Result{r}
+		f(q, resultExpected)
+	})
+	t.Run("time() offset -1h40s0ms", func(t *testing.T) {
+		t.Parallel()
+		q := `time() offset -1h40s0ms`
+		r := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{4600, 4800, 5000, 5200, 5400, 5600},
 			Timestamps: timestampsExpected,
 		}
 		resultExpected := []netstorage.Result{r}
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index a1fdec7beb..400851c783 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 * FEATURE: do not publish duplicate Docker images with `-cluster` tag suffix for [vmagent](https://victoriametrics.github.io/vmagent.html), [vmalert](https://victoriametrics.github.io/vmalert.html), [vmauth](https://victoriametrics.github.io/vmauth.html), [vmbackup](https://victoriametrics.github.io/vmbackup.html) and [vmrestore](https://victoriametrics.github.io/vmrestore.html), since they are identical to images without `-cluster` tag suffix.
 
+* BUGFIX: properly parse negative combined durations in MetricsQL such as `-1h3m4s`. It must be parsed as `-(1h + 3m + 4s)`. Prevsiously it was parsed as `-1h + 3m + 4s`.
 * BUGFIX: properly parse lines in [Prometheus exposition format](https://github.com/prometheus/docs/blob/master/content/docs/instrumenting/exposition_formats.md) and in [OpenMetrics format](https://github.com/OpenObservability/OpenMetrics/blob/master/specification/OpenMetrics.md) with whitespace after the timestamp. For example, `foo 123 456 # some comment here`. See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/970
 
 
diff --git a/go.mod b/go.mod
index cfd6dd7037..431509e5cf 100644
--- a/go.mod
+++ b/go.mod
@@ -9,7 +9,7 @@ require (
 	// like https://github.com/valyala/fasthttp/commit/996610f021ff45fdc98c2ce7884d5fa4e7f9199b
 	github.com/VictoriaMetrics/fasthttp v1.0.9
 	github.com/VictoriaMetrics/metrics v1.12.3
-	github.com/VictoriaMetrics/metricsql v0.9.0
+	github.com/VictoriaMetrics/metricsql v0.9.1
 	github.com/aws/aws-sdk-go v1.36.7
 	github.com/cespare/xxhash/v2 v2.1.1
 	github.com/golang/snappy v0.0.2
diff --git a/go.sum b/go.sum
index 13d027903f..39680720d9 100644
--- a/go.sum
+++ b/go.sum
@@ -46,8 +46,8 @@ github.com/VictoriaMetrics/fasthttp v1.0.9/go.mod h1:3SeUL4zwB/p/a9aEeRc6gdlbrtN
 github.com/VictoriaMetrics/metrics v1.12.2/go.mod h1:Z1tSfPfngDn12bTfZSCqArT3OPY3u88J12hSoOhuiRE=
 github.com/VictoriaMetrics/metrics v1.12.3 h1:Fe6JHC6MSEKa+BtLhPN8WIvS+HKPzMc2evEpNeCGy7I=
 github.com/VictoriaMetrics/metrics v1.12.3/go.mod h1:Z1tSfPfngDn12bTfZSCqArT3OPY3u88J12hSoOhuiRE=
-github.com/VictoriaMetrics/metricsql v0.9.0 h1:mO4YmVRVHQmipTHcSMlCJ7Rctsol7vlu1l2ifh+ibqI=
-github.com/VictoriaMetrics/metricsql v0.9.0/go.mod h1:ylO7YITho/Iw6P71oEaGyHbO94bGoGtzWfLGqFhMIg8=
+github.com/VictoriaMetrics/metricsql v0.9.1 h1:CVl9fSW4pGhv7r9Q54zBPVVIGmwpAWvfo0QybVv+TV8=
+github.com/VictoriaMetrics/metricsql v0.9.1/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=
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/lexer.go b/vendor/github.com/VictoriaMetrics/metricsql/lexer.go
index f607e47a99..c429a2fca2 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/lexer.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/lexer.go
@@ -408,8 +408,12 @@ func isPositiveDuration(s string) bool {
 	return n == len(s)
 }
 
-// PositiveDurationValue returns the duration in milliseconds for the given s
+// PositiveDurationValue returns positive duration in milliseconds for the given s
 // and the given step.
+//
+// Duration in s may be combined, i.e. 2h5m or 2h-5m.
+//
+// Error is returned if the duration in s is negative.
 func PositiveDurationValue(s string, step int64) (int64, error) {
 	d, err := DurationValue(s, step)
 	if err != nil {
@@ -424,7 +428,7 @@ func PositiveDurationValue(s string, step int64) (int64, error) {
 // DurationValue returns the duration in milliseconds for the given s
 // and the given step.
 //
-// Duration in s may be combined, i.e. 2h5m or 2h-5m.
+// Duration in s may be combined, i.e. 2h5m, -2h5m or 2h-5m.
 //
 // The returned duration value can be negative.
 func DurationValue(s string, step int64) (int64, error) {
@@ -432,6 +436,7 @@ func DurationValue(s string, step int64) (int64, error) {
 		return 0, fmt.Errorf("duration cannot be empty")
 	}
 	var d int64
+	isMinus := false
 	for len(s) > 0 {
 		n := scanSingleDuration(s, true)
 		if n <= 0 {
@@ -443,7 +448,13 @@ func DurationValue(s string, step int64) (int64, error) {
 		if err != nil {
 			return 0, err
 		}
+		if isMinus && dLocal > 0 {
+			dLocal = -dLocal
+		}
 		d += dLocal
+		if dLocal < 0 {
+			isMinus = true
+		}
 	}
 	return d, nil
 }
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 88d16e8876..959f59e8eb 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -16,7 +16,7 @@ github.com/VictoriaMetrics/fasthttp/fasthttputil
 github.com/VictoriaMetrics/fasthttp/stackless
 # github.com/VictoriaMetrics/metrics v1.12.3
 github.com/VictoriaMetrics/metrics
-# github.com/VictoriaMetrics/metricsql v0.9.0
+# github.com/VictoriaMetrics/metricsql v0.9.1
 github.com/VictoriaMetrics/metricsql
 github.com/VictoriaMetrics/metricsql/binaryop
 # github.com/aws/aws-sdk-go v1.36.7