Skip to content

Commit

Permalink
Fix parseFill to check for fill ident before attempting to parse an e…
Browse files Browse the repository at this point in the history
…xpression

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.
  • Loading branch information
jsternberg committed Aug 1, 2016
1 parent cac6765 commit 2c739c0
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
25 changes: 12 additions & 13 deletions influxql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2071,32 +2071,31 @@ 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":
return NoFill, nil, nil
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:
Expand Down
1 change: 1 addition & 0 deletions influxql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`},
Expand Down

0 comments on commit 2c739c0

Please sign in to comment.