From ccf0cb83716e9ff48be59caebed3f2e57a560a85 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Wed, 22 Mar 2017 14:21:34 -0500 Subject: [PATCH] Fix query parser when using addition and subtraction without spaces Additionally, support unary addition and subtraction for variables, calls, and parenthesis expressions. Doing `-value` will be the equivalent of doing `-1 * value` now. --- CHANGELOG.md | 1 + influxql/README.md | 4 +- influxql/parser.go | 38 +++++++++++++++++++ influxql/parser_test.go | 81 ++++++++++++++++++++++++++++++++++++++++ influxql/scanner.go | 38 ++++++------------- influxql/scanner_test.go | 10 +---- 6 files changed, 135 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8c2e1da97..1dc25d5826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - [#8118](https://github.com/influxdata/influxdb/issues/8118): Significantly improve DROP DATABASE speed. - [#8181](https://github.com/influxdata/influxdb/issues/8181): Return an error when an invalid duration literal is parsed. - [#8093](https://github.com/influxdata/influxdb/issues/8093): Fix the time range when an exact timestamp is selected. +- [#8174](https://github.com/influxdata/influxdb/issues/8174): Fix query parser when using addition and subtraction without spaces. ## v1.2.2 [2017-03-14] diff --git a/influxql/README.md b/influxql/README.md index 9da91cf5de..8778c5e1c5 100644 --- a/influxql/README.md +++ b/influxql/README.md @@ -136,7 +136,7 @@ InfluxQL supports decimal integer literals. Hexadecimal and octal literals are not currently supported. ``` -int_lit = ( "1" … "9" ) { digit } . +int_lit = [ "+" | "-" ] ( "1" … "9" ) { digit } . ``` ### Floats @@ -144,7 +144,7 @@ int_lit = ( "1" … "9" ) { digit } . InfluxQL supports floating-point literals. Exponents are not currently supported. ``` -float_lit = int_lit "." int_lit . +float_lit = [ "+" | "-" ] ( "." digit { digit } | digit { digit } "." { digit } ) . ``` ### Strings diff --git a/influxql/parser.go b/influxql/parser.go index 592bec28a5..44d7eed337 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -2549,6 +2549,44 @@ func (p *Parser) parseUnaryExpr() (Expr, error) { default: return nil, fmt.Errorf("unable to bind parameter with type %T", v) } + case ADD, SUB: + mul := 1 + if tok == SUB { + mul = -1 + } + + tok0, pos0, lit0 := p.scanIgnoreWhitespace() + switch tok0 { + case NUMBER, INTEGER, DURATIONVAL, LPAREN, IDENT: + // Unscan the token and use parseUnaryExpr. + p.unscan() + + lit, err := p.parseUnaryExpr() + if err != nil { + return nil, err + } + + switch lit := lit.(type) { + case *NumberLiteral: + lit.Val *= float64(mul) + case *IntegerLiteral: + lit.Val *= int64(mul) + case *DurationLiteral: + lit.Val *= time.Duration(mul) + case *VarRef, *Call, *ParenExpr: + // Multiply the variable. + return &BinaryExpr{ + Op: MUL, + LHS: &IntegerLiteral{Val: int64(mul)}, + RHS: lit, + }, nil + default: + panic(fmt.Sprintf("unexpected literal: %T", lit)) + } + return lit, nil + default: + return nil, newParseError(tokstr(tok0, lit0), []string{"identifier", "number", "duration", "("}, pos0) + } default: return nil, newParseError(tokstr(tok, lit), []string{"identifier", "string", "number", "bool"}, pos) } diff --git a/influxql/parser_test.go b/influxql/parser_test.go index dbf8f4b400..066a169c99 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -2827,6 +2827,15 @@ func TestParser_ParseExpr(t *testing.T) { // Primitives {s: `100.0`, expr: &influxql.NumberLiteral{Val: 100}}, {s: `100`, expr: &influxql.IntegerLiteral{Val: 100}}, + {s: `-100.0`, expr: &influxql.NumberLiteral{Val: -100}}, + {s: `-100`, expr: &influxql.IntegerLiteral{Val: -100}}, + {s: `100.`, expr: &influxql.NumberLiteral{Val: 100}}, + {s: `-100.`, expr: &influxql.NumberLiteral{Val: -100}}, + {s: `.23`, expr: &influxql.NumberLiteral{Val: 0.23}}, + {s: `-.23`, expr: &influxql.NumberLiteral{Val: -0.23}}, + {s: `1s`, expr: &influxql.DurationLiteral{Val: time.Second}}, + {s: `-1s`, expr: &influxql.DurationLiteral{Val: -time.Second}}, + {s: `-+1`, err: `found +, expected identifier, number, duration, ( at line 1, char 2`}, {s: `'foo bar'`, expr: &influxql.StringLiteral{Val: "foo bar"}}, {s: `true`, expr: &influxql.BooleanLiteral{Val: true}}, {s: `false`, expr: &influxql.BooleanLiteral{Val: false}}, @@ -2958,6 +2967,78 @@ func TestParser_ParseExpr(t *testing.T) { }, }, + // Addition and subtraction without whitespace. + { + s: `1+2-3`, + expr: &influxql.BinaryExpr{ + Op: influxql.SUB, + LHS: &influxql.BinaryExpr{ + Op: influxql.ADD, + LHS: &influxql.IntegerLiteral{Val: 1}, + RHS: &influxql.IntegerLiteral{Val: 2}, + }, + RHS: &influxql.IntegerLiteral{Val: 3}, + }, + }, + + { + s: `time>now()-5m`, + expr: &influxql.BinaryExpr{ + Op: influxql.GT, + LHS: &influxql.VarRef{Val: "time"}, + RHS: &influxql.BinaryExpr{ + Op: influxql.SUB, + LHS: &influxql.Call{Name: "now"}, + RHS: &influxql.DurationLiteral{Val: 5 * time.Minute}, + }, + }, + }, + + // Simple unary expression. + { + s: `-value`, + expr: &influxql.BinaryExpr{ + Op: influxql.MUL, + LHS: &influxql.IntegerLiteral{Val: -1}, + RHS: &influxql.VarRef{Val: "value"}, + }, + }, + + { + s: `-mean(value)`, + expr: &influxql.BinaryExpr{ + Op: influxql.MUL, + LHS: &influxql.IntegerLiteral{Val: -1}, + RHS: &influxql.Call{ + Name: "mean", + Args: []influxql.Expr{ + &influxql.VarRef{Val: "value"}}, + }, + }, + }, + + // Unary expressions with parenthesis. + { + s: `-(-4)`, + expr: &influxql.BinaryExpr{ + Op: influxql.MUL, + LHS: &influxql.IntegerLiteral{Val: -1}, + RHS: &influxql.ParenExpr{ + Expr: &influxql.IntegerLiteral{Val: -4}, + }, + }, + }, + + // Multiplication with leading subtraction. + { + s: `-2 * 3`, + expr: &influxql.BinaryExpr{ + Op: influxql.MUL, + LHS: &influxql.IntegerLiteral{Val: -2}, + RHS: &influxql.IntegerLiteral{Val: 3}, + }, + }, + // Binary expression with regex. { s: `region =~ /us.*/`, diff --git a/influxql/scanner.go b/influxql/scanner.go index a4bda50ff7..1fa8a951ac 100644 --- a/influxql/scanner.go +++ b/influxql/scanner.go @@ -59,8 +59,16 @@ func (s *Scanner) Scan() (tok Token, pos Pos, lit string) { return tok, pos, "$" + lit } return BOUNDPARAM, pos, "$" + lit - case '+', '-': - return s.scanNumber() + case '+': + return ADD, pos, "" + case '-': + ch1, _ := s.r.read() + if ch1 == '-' { + s.skipUntilNewline() + return COMMENT, pos, "" + } + s.r.unread() + return SUB, pos, "" case '*': return MUL, pos, "" case '/': @@ -248,34 +256,12 @@ func (s *Scanner) ScanRegex() (tok Token, pos Pos, lit string) { } // scanNumber consumes anything that looks like the start of a number. -// Numbers start with a digit, full stop, plus sign or minus sign. -// This function can return non-number tokens if a scan is a false positive. -// For example, a minus sign followed by a letter will just return a minus sign. func (s *Scanner) scanNumber() (tok Token, pos Pos, lit string) { var buf bytes.Buffer - // Check if the initial rune is a "+" or "-". + // Check if the initial rune is a ".". ch, pos := s.r.curr() - if ch == '+' || ch == '-' { - // Peek at the next two runes. - ch1, _ := s.r.read() - ch2, _ := s.r.read() - s.r.unread() - s.r.unread() - - // This rune must be followed by a digit or a full stop and a digit. - if isDigit(ch1) || (ch1 == '.' && isDigit(ch2)) { - _, _ = buf.WriteRune(ch) - } else if ch == '+' { - return ADD, pos, "" - } else if ch == '-' { - if ch1 == '-' { - s.skipUntilNewline() - return COMMENT, pos, "" - } - return SUB, pos, "" - } - } else if ch == '.' { + if ch == '.' { // Peek and see if the next rune is a digit. ch1, _ := s.r.read() s.r.unread() diff --git a/influxql/scanner_test.go b/influxql/scanner_test.go index dd14340c60..ad24359d7f 100644 --- a/influxql/scanner_test.go +++ b/influxql/scanner_test.go @@ -87,24 +87,16 @@ func TestScanner_Scan(t *testing.T) { // Numbers {s: `100`, tok: influxql.INTEGER, lit: `100`}, - {s: `-100`, tok: influxql.INTEGER, lit: `-100`}, {s: `100.23`, tok: influxql.NUMBER, lit: `100.23`}, - {s: `+100.23`, tok: influxql.NUMBER, lit: `+100.23`}, - {s: `-100.23`, tok: influxql.NUMBER, lit: `-100.23`}, - {s: `-100.`, tok: influxql.NUMBER, lit: `-100`}, {s: `.23`, tok: influxql.NUMBER, lit: `.23`}, - {s: `+.23`, tok: influxql.NUMBER, lit: `+.23`}, - {s: `-.23`, tok: influxql.NUMBER, lit: `-.23`}, //{s: `.`, tok: influxql.ILLEGAL, lit: `.`}, - {s: `-.`, tok: influxql.SUB, lit: ``}, - {s: `+.`, tok: influxql.ADD, lit: ``}, {s: `10.3s`, tok: influxql.NUMBER, lit: `10.3`}, // Durations {s: `10u`, tok: influxql.DURATIONVAL, lit: `10u`}, {s: `10µ`, tok: influxql.DURATIONVAL, lit: `10µ`}, {s: `10ms`, tok: influxql.DURATIONVAL, lit: `10ms`}, - {s: `-1s`, tok: influxql.DURATIONVAL, lit: `-1s`}, + {s: `1s`, tok: influxql.DURATIONVAL, lit: `1s`}, {s: `10m`, tok: influxql.DURATIONVAL, lit: `10m`}, {s: `10h`, tok: influxql.DURATIONVAL, lit: `10h`}, {s: `10d`, tok: influxql.DURATIONVAL, lit: `10d`},