From 73db8b8cea3261c1ea10df787c0fc8646e8ee595 Mon Sep 17 00:00:00 2001
From: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Date: Thu, 13 Sep 2018 13:38:01 +0530
Subject: [PATCH] [bugfix] Parse negative value in PromQL (#4564)

* Parse negative value in PromQL
* Enforce space between values

Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
---
 promql/lex.go        |  3 +++
 promql/lex_test.go   |  8 ++++++++
 promql/parse.go      | 17 ++++++++++++++++-
 promql/parse_test.go | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/promql/lex.go b/promql/lex.go
index bb2476cf6..4a8130491 100644
--- a/promql/lex.go
+++ b/promql/lex.go
@@ -143,6 +143,7 @@ const (
 	itemDuration
 	itemBlank
 	itemTimes
+	itemSpace
 
 	operatorsStart
 	// Operators.
@@ -247,6 +248,7 @@ var itemTypeStr = map[ItemType]string{
 	itemSemicolon:    ";",
 	itemBlank:        "_",
 	itemTimes:        "x",
+	itemSpace:        "<space>",
 
 	itemSUB:      "-",
 	itemADD:      "+",
@@ -616,6 +618,7 @@ func lexValueSequence(l *lexer) stateFn {
 	case r == eof:
 		return lexStatements
 	case isSpace(r):
+		l.emit(itemSpace)
 		lexSpace(l)
 	case r == '+':
 		l.emit(itemADD)
diff --git a/promql/lex_test.go b/promql/lex_test.go
index 1c49d19bd..333e3b7a0 100644
--- a/promql/lex_test.go
+++ b/promql/lex_test.go
@@ -406,9 +406,13 @@ var tests = []struct {
 		expected: []item{
 			{itemLeftBrace, 0, `{`},
 			{itemRightBrace, 1, `}`},
+			{itemSpace, 2, ` `},
 			{itemBlank, 3, `_`},
+			{itemSpace, 4, ` `},
 			{itemNumber, 5, `1`},
+			{itemSpace, 6, ` `},
 			{itemTimes, 7, `x`},
+			{itemSpace, 8, ` `},
 			{itemNumber, 9, `.3`},
 		},
 		seriesDesc: true,
@@ -417,9 +421,12 @@ var tests = []struct {
 		input: `metric +Inf Inf NaN`,
 		expected: []item{
 			{itemIdentifier, 0, `metric`},
+			{itemSpace, 6, ` `},
 			{itemADD, 7, `+`},
 			{itemNumber, 8, `Inf`},
+			{itemSpace, 11, ` `},
 			{itemNumber, 12, `Inf`},
+			{itemSpace, 15, ` `},
 			{itemNumber, 16, `NaN`},
 		},
 		seriesDesc: true,
@@ -428,6 +435,7 @@ var tests = []struct {
 		input: `metric 1+1x4`,
 		expected: []item{
 			{itemIdentifier, 0, `metric`},
+			{itemSpace, 6, ` `},
 			{itemNumber, 7, `1`},
 			{itemADD, 8, `+`},
 			{itemNumber, 9, `1`},
diff --git a/promql/parse.go b/promql/parse.go
index f370f7831..5fe2042b2 100644
--- a/promql/parse.go
+++ b/promql/parse.go
@@ -175,6 +175,9 @@ func (p *parser) parseSeriesDesc() (m labels.Labels, vals []sequenceValue, err e
 
 	const ctx = "series values"
 	for {
+		for p.peek().typ == itemSpace {
+			p.next()
+		}
 		if p.peek().typ == itemEOF {
 			break
 		}
@@ -193,6 +196,11 @@ func (p *parser) parseSeriesDesc() (m labels.Labels, vals []sequenceValue, err e
 			for i := uint64(0); i < times; i++ {
 				vals = append(vals, sequenceValue{omitted: true})
 			}
+			// This is to ensure that there is a space between this and the next number.
+			// This is especially required if the next number is negative.
+			if t := p.expectOneOf(itemSpace, itemEOF, ctx).typ; t == itemEOF {
+				break
+			}
 			continue
 		}
 
@@ -217,7 +225,8 @@ func (p *parser) parseSeriesDesc() (m labels.Labels, vals []sequenceValue, err e
 		})
 
 		// If there are no offset repetitions specified, proceed with the next value.
-		if t := p.peek(); t.typ == itemNumber || t.typ == itemBlank || t.typ == itemIdentifier && t.val == "stale" {
+		if t := p.peek(); t.typ == itemSpace {
+			// This ensures there is a space between every value.
 			continue
 		} else if t.typ == itemEOF {
 			break
@@ -244,6 +253,12 @@ func (p *parser) parseSeriesDesc() (m labels.Labels, vals []sequenceValue, err e
 				value: k,
 			})
 		}
+		// This is to ensure that there is a space between this expanding notation
+		// and the next number. This is especially required if the next number
+		// is negative.
+		if t := p.expectOneOf(itemSpace, itemEOF, ctx).typ; t == itemEOF {
+			break
+		}
 	}
 	return m, vals, nil
 }
diff --git a/promql/parse_test.go b/promql/parse_test.go
index a17d7af44..18b54b824 100644
--- a/promql/parse_test.go
+++ b/promql/parse_test.go
@@ -1737,6 +1737,39 @@ var testSeries = []struct {
 	}, {
 		input: `my_metric{a="b"} 1 3 _ 5 _a4`,
 		fail:  true,
+	}, {
+		input:          `my_metric{a="b"} 1 -1`,
+		expectedMetric: labels.FromStrings(labels.MetricName, "my_metric", "a", "b"),
+		expectedValues: newSeq(1, -1),
+	}, {
+		input:          `my_metric{a="b"} 1 +1`,
+		expectedMetric: labels.FromStrings(labels.MetricName, "my_metric", "a", "b"),
+		expectedValues: newSeq(1, 1),
+	}, {
+		input:          `my_metric{a="b"} 1 -1 -3-10x4 7 9 +5`,
+		expectedMetric: labels.FromStrings(labels.MetricName, "my_metric", "a", "b"),
+		expectedValues: newSeq(1, -1, -3, -13, -23, -33, -43, 7, 9, 5),
+	}, {
+		input:          `my_metric{a="b"} 1 +1 +4 -6 -2 8`,
+		expectedMetric: labels.FromStrings(labels.MetricName, "my_metric", "a", "b"),
+		expectedValues: newSeq(1, 1, 4, -6, -2, 8),
+	}, {
+		// Trailing spaces should be correctly handles.
+		input:          `my_metric{a="b"} 1 2 3    `,
+		expectedMetric: labels.FromStrings(labels.MetricName, "my_metric", "a", "b"),
+		expectedValues: newSeq(1, 2, 3),
+	}, {
+		input: `my_metric{a="b"} -3-3 -3`,
+		fail:  true,
+	}, {
+		input: `my_metric{a="b"} -3 -3-3`,
+		fail:  true,
+	}, {
+		input: `my_metric{a="b"} -3 _-2`,
+		fail:  true,
+	}, {
+		input: `my_metric{a="b"} -3 3+3x4-4`,
+		fail:  true,
 	},
 }