Skip to content

Commit

Permalink
check if fields are valid during parse.
Browse files Browse the repository at this point in the history
Binary expressions that yield a boolean are invalid and we can catch
these at parse time.

Fixes influxdata#3525
  • Loading branch information
DanielMorsing committed Aug 4, 2015
1 parent 4af2272 commit 6797270
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- [#3420](https://github.com/influxdb/influxdb/pull/3420): Catch opentsdb malformed tags. Thanks @nathanielc.
- [#3404](https://github.com/influxdb/influxdb/pull/3404): Added support for escaped single quotes in query string. Thanks @jhorwit2
- [#3414](https://github.com/influxdb/influxdb/issues/3414): Shard mappers perform query re-writing
- [#3525](https://github.com/influxdb/influxdb/pull/3525): check if fields are valid during parse time.

## v0.9.2 [2015-07-24]

Expand Down
31 changes: 31 additions & 0 deletions influxql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -1471,11 +1471,18 @@ func (p *Parser) parseFields() (Fields, error) {
func (p *Parser) parseField() (*Field, error) {
f := &Field{}

_, pos, _ := p.scanIgnoreWhitespace()
p.unscan()
// Parse the expression first.
expr, err := p.ParseExpr()
if err != nil {
return nil, err
}
var c validateField
Walk(&c, expr)
if c.foundInvalid {
return nil, fmt.Errorf("invalid operator %s in SELECT clause at line %d, char %d; operator is intended for WHERE clause", c.badToken, pos.Line+1, pos.Char+1)
}
f.Expr = expr

// Parse the alias if the current and next tokens are "WS AS".
Expand All @@ -1491,6 +1498,30 @@ func (p *Parser) parseField() (*Field, error) {
return f, nil
}

// validateField checks if the Expr is a valid field. We disallow all binary expression
// that return a boolean
type validateField struct {
foundInvalid bool
badToken Token
}

func (c *validateField) Visit(n Node) Visitor {
e, ok := n.(*BinaryExpr)
if !ok {
return c
}

switch e.Op {
case EQ, NEQ, EQREGEX,
NEQREGEX, LT, LTE, GT, GTE,
AND, OR:
c.foundInvalid = true
c.badToken = e.Op
return nil
}
return c
}

// parseAlias parses the "AS (IDENT|STRING)" alias for fields and dimensions.
func (p *Parser) parseAlias() (string, error) {
// Check if the next token is "AS". If not, then unscan and exit.
Expand Down
3 changes: 3 additions & 0 deletions influxql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,9 @@ func TestParser_ParseStatement(t *testing.T) {
{s: `select derivative() from myseries`, err: `invalid number of arguments for derivative, expected at least 1 but no more than 2, got 0`},
{s: `select derivative(mean(value), 1h, 3) from myseries`, err: `invalid number of arguments for derivative, expected at least 1 but no more than 2, got 3`},
{s: `SELECT field1 from myseries WHERE host =~ 'asd' LIMIT 1`, err: `found asd, expected regex at line 1, char 42`},
{s: `SELECT value > 2 FROM cpu`, err: `invalid operator > in SELECT clause at line 1, char 8; operator is intended for WHERE clause`},
{s: `SELECT value = 2 FROM cpu`, err: `invalid operator = in SELECT clause at line 1, char 8; operator is intended for WHERE clause`},
{s: `SELECT s =~ /foo/ FROM cpu`, err: `invalid operator =~ in SELECT clause at line 1, char 8; operator is intended for WHERE clause`},
{s: `DELETE`, err: `found EOF, expected FROM at line 1, char 8`},
{s: `DELETE FROM`, err: `found EOF, expected identifier at line 1, char 13`},
{s: `DELETE FROM myseries WHERE`, err: `found EOF, expected identifier, string, number, bool at line 1, char 28`},
Expand Down

0 comments on commit 6797270

Please sign in to comment.