fix #2268: fix expression parsing bug

CHANGELOG.md

@@ -4,6 +4,7 @@
 - [#2282](https://github.com/influxdb/influxdb/pull/2282): Use "value" as field name for OpenTSDB input.
 - [#2283](https://github.com/influxdb/influxdb/pull/2283): Fix bug when restarting an entire existing cluster.
 - [#2287](https://github.com/influxdb/influxdb/pull/2287): Fix data race during SHOW RETENTION POLICIES.
+- [#2288](https://github.com/influxdb/influxdb/pull/2288): Fix expression parsing bug.
 
 ## Features
 - [#2276](https://github.com/influxdb/influxdb/pull/2276): Broker topic truncation.

influxql/parser.go

@@ -1706,10 +1706,12 @@ func (p *Parser) parseVarRef() (*VarRef, error) {
 // ParseExpr parses an expression.
 func (p *Parser) ParseExpr() (Expr, error) {
 	var err error
+	// Dummy root node.
+	root := &BinaryExpr{}
 
 	// Parse a non-binary expression type to start.
 	// This variable will always be the root of the expression tree.
-	expr, err := p.parseUnaryExpr()
+	root.RHS, err = p.parseUnaryExpr()
 	if err != nil {
 		return nil, err
 	}
@@ -1720,7 +1722,7 @@ func (p *Parser) ParseExpr() (Expr, error) {
 		op, _, _ := p.scanIgnoreWhitespace()
 		if !op.isOperator() {
 			p.unscan()
-			return expr, nil
+			return root.RHS, nil
 		}
 
 		// Otherwise parse the next expression.
@@ -1737,15 +1739,18 @@ func (p *Parser) ParseExpr() (Expr, error) {
 			}
 		}
 
-		// Assign the new root based on the precendence of the LHS and RHS operators.
-		if lhs, ok := expr.(*BinaryExpr); ok && lhs.Op.Precedence() < op.Precedence() {
-			expr = &BinaryExpr{
-				LHS: lhs.LHS,
-				RHS: &BinaryExpr{LHS: lhs.RHS, RHS: rhs, Op: op},
-				Op:  lhs.Op,
+		// Find the right spot in the tree to add the new expression by
+		// descending the RHS of the expression tree until we reach the last
+		// BinaryExpr or a BinaryExpr whose RHS has an operator with
+		// precedence >= the operator being added.
+		for node := root; ; {
+			r, ok := node.RHS.(*BinaryExpr)
+			if !ok || r.Op.Precedence() >= op.Precedence() {
+				// Add the new expression here and break.
+				node.RHS = &BinaryExpr{LHS: node.RHS, RHS: rhs, Op: op}
+				break
 			}
-		} else {
-			expr = &BinaryExpr{LHS: expr, RHS: rhs, Op: op}
+			node = r
 		}
 	}
 }

influxql/parser_test.go

@@ -1172,6 +1172,32 @@ func TestParser_ParseExpr(t *testing.T) {
 			},
 		},
 
+		// Complex binary expression.
+		{
+			s: `time > now() - 1d AND time < now() + 1d`,
+			expr: &influxql.BinaryExpr{
+				Op: influxql.AND,
+				LHS: &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: mustParseDuration("1d")},
+					},
+				},
+				RHS: &influxql.BinaryExpr{
+					Op:  influxql.LT,
+					LHS: &influxql.VarRef{Val: "time"},
+					RHS: &influxql.BinaryExpr{
+						Op:  influxql.ADD,
+						LHS: &influxql.Call{Name: "now"},
+						RHS: &influxql.DurationLiteral{Val: mustParseDuration("1d")},
+					},
+				},
+			},
+		},
+
 		// Function call (empty)
 		{
 			s: `my_func()`,
@@ -1375,18 +1401,14 @@ func BenchmarkParserParseStatement(b *testing.B) {
 // MustParseSelectStatement parses a select statement. Panic on error.
 func MustParseSelectStatement(s string) *influxql.SelectStatement {
 	stmt, err := influxql.NewParser(strings.NewReader(s)).ParseStatement()
-	if err != nil {
-		panic(err.Error())
-	}
+	panicIfErr(err)
 	return stmt.(*influxql.SelectStatement)
 }
 
 // MustParseExpr parses an expression. Panic on error.
 func MustParseExpr(s string) influxql.Expr {
 	expr, err := influxql.NewParser(strings.NewReader(s)).ParseExpr()
-	if err != nil {
-		panic(err.Error())
-	}
+	panicIfErr(err)
 	return expr
 }
 
@@ -1420,8 +1442,18 @@ func newAlterRetentionPolicyStatement(name string, DB string, d time.Duration, r
 // mustMarshalJSON encodes a value to JSON.
 func mustMarshalJSON(v interface{}) []byte {
 	b, err := json.Marshal(v)
+	panicIfErr(err)
+	return b
+}
+
+func mustParseDuration(s string) time.Duration {
+	d, err := influxql.ParseDuration(s)
+	panicIfErr(err)
+	return d
+}
+
+func panicIfErr(err error) {
 	if err != nil {
-		panic("marshal json: " + err.Error())
+		panic(err)
 	}
-	return b
 }