From 2c739c0532d047a4eb7f8cfd689a8332ceea2e02 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Mon, 1 Aug 2016 11:38:42 -0500 Subject: [PATCH] Fix parseFill to check for fill ident before attempting to parse an expression The previous parseFill would try to parse an expression and only unscan one token when it failed. This caused it to not put back the correct number of tokens with some expression. Now it has been modified to check for the fill ident ahead of time and then use ParseExpr() to parse the call. If the expression fails to parse into a call, it will send an error instead of trying to continue with an invalid parser state. Fixes #6543. --- CHANGELOG.md | 1 + influxql/parser.go | 25 ++++++++++++------------- influxql/parser_test.go | 1 + 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33a87abf852..845c6050a86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -112,6 +112,7 @@ With this release the systemd configuration files for InfluxDB will use the syst - [#7080](https://github.com/influxdata/influxdb/pull/7080): Ensure IDs can't clash when managing Continuous Queries. - [#6990](https://github.com/influxdata/influxdb/issues/6990): Fix panic parsing empty key - [#7084](https://github.com/influxdata/influxdb/pull/7084): Tombstone memory improvements +- [#6543](https://github.com/influxdata/influxdb/issues/6543): Fix parseFill to check for fill ident before attempting to parse an expression. ## v0.13.0 [2016-05-12] diff --git a/influxql/parser.go b/influxql/parser.go index 049b8021af7..c732441e90c 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -2071,24 +2071,23 @@ func (p *Parser) parseDimension() (*Dimension, error) { // parseFill parses the fill call and its options. func (p *Parser) parseFill() (FillOption, interface{}, error) { // Parse the expression first. + tok, _, lit := p.scanIgnoreWhitespace() + p.unscan() + if tok != IDENT || strings.ToLower(lit) != "fill" { + return NullFill, nil, nil + } + expr, err := p.ParseExpr() if err != nil { - p.unscan() - return NullFill, nil, nil + return NullFill, nil, err } - lit, ok := expr.(*Call) + fill, ok := expr.(*Call) if !ok { - p.unscan() - return NullFill, nil, nil - } - if strings.ToLower(lit.Name) != "fill" { - p.unscan() - return NullFill, nil, nil - } - if len(lit.Args) != 1 { + return NullFill, nil, errors.New("fill must be a function call") + } else if len(fill.Args) != 1 { return NullFill, nil, errors.New("fill requires an argument, e.g.: 0, null, none, previous") } - switch lit.Args[0].String() { + switch fill.Args[0].String() { case "null": return NullFill, nil, nil case "none": @@ -2096,7 +2095,7 @@ func (p *Parser) parseFill() (FillOption, interface{}, error) { case "previous": return PreviousFill, nil, nil default: - switch num := lit.Args[0].(type) { + switch num := fill.Args[0].(type) { case *IntegerLiteral: return NumberFill, num.Val, nil case *NumberLiteral: diff --git a/influxql/parser_test.go b/influxql/parser_test.go index 4acd7d027d1..b514cb57f7f 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -2152,6 +2152,7 @@ func TestParser_ParseStatement(t *testing.T) { {s: `SELECT count(foo + sum(bar)) FROM cpu`, err: `expected field argument in count()`}, {s: `SELECT (count(foo + sum(bar))) FROM cpu`, err: `expected field argument in count()`}, {s: `SELECT sum(value) + count(foo + sum(bar)) FROM cpu`, err: `binary expressions cannot mix aggregates and raw fields`}, + {s: `SELECT mean(value) FROM cpu FILL + value`, err: `fill must be a function call`}, // See issues https://github.com/influxdata/influxdb/issues/1647 // and https://github.com/influxdata/influxdb/issues/4404 //{s: `DELETE`, err: `found EOF, expected FROM at line 1, char 8`},