From 2062fbae0fbbd9f964d9c10ae08008cabb45fe39 Mon Sep 17 00:00:00 2001 From: Patrick Bogen Date: Wed, 2 Mar 2016 15:56:40 -0800 Subject: [PATCH 1/2] rewrite operator balancing to be recursive --- promql/parse.go | 55 ++++++++++++++++++++++---------------------- promql/parse_test.go | 14 +++++++++++ 2 files changed, 41 insertions(+), 28 deletions(-) diff --git a/promql/parse.go b/promql/parse.go index f71447893..91fb6503f 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -530,35 +530,34 @@ func (p *parser) expr() Expr { rhs := p.unaryExpr() // Assign the new root based on the precedence of the LHS and RHS operators. - if lhs, ok := expr.(*BinaryExpr); ok && lhs.Op.precedence() < op.precedence() { - expr = &BinaryExpr{ - Op: lhs.Op, - LHS: lhs.LHS, - RHS: &BinaryExpr{ - Op: op, - LHS: lhs.RHS, - RHS: rhs, - VectorMatching: vecMatching, - ReturnBool: returnBool, - }, - VectorMatching: lhs.VectorMatching, - } - if op.isComparisonOperator() && !returnBool && rhs.Type() == model.ValScalar && lhs.RHS.Type() == model.ValScalar { - p.errorf("comparisons between scalars must use BOOL modifier") - } - } else { - expr = &BinaryExpr{ - Op: op, - LHS: expr, - RHS: rhs, - VectorMatching: vecMatching, - ReturnBool: returnBool, - } - if op.isComparisonOperator() && !returnBool && rhs.Type() == model.ValScalar && expr.Type() == model.ValScalar { - p.errorf("comparisons between scalars must use BOOL modifier") - } - } + expr = p.balance(expr, op, rhs, vecMatching, returnBool) + } +} +func (p *parser) balance(lhs Expr, op itemType, rhs Expr, vecMatching *VectorMatching, returnBool bool) *BinaryExpr { + if lhsBE, ok := lhs.(*BinaryExpr); ok && lhsBE.Op.precedence() < op.precedence() { + var balanced = p.balance(lhsBE.RHS, op, rhs, vecMatching, returnBool) + if lhsBE.Op.isComparisonOperator() && !lhsBE.ReturnBool && balanced.Type() == model.ValScalar && lhsBE.LHS.Type() == model.ValScalar { + p.errorf("comparisons between scalars must use BOOL modifier") + } + return &BinaryExpr{ + Op: lhsBE.Op, + LHS: lhsBE.LHS, + RHS: balanced, + VectorMatching: lhsBE.VectorMatching, + ReturnBool: lhsBE.ReturnBool, + } + } else { + if op.isComparisonOperator() && !returnBool && rhs.Type() == model.ValScalar && lhs.Type() == model.ValScalar { + p.errorf("comparisons between scalars must use BOOL modifier") + } + return &BinaryExpr{ + Op: op, + LHS: lhs, + RHS: rhs, + VectorMatching: vecMatching, + ReturnBool: returnBool, + } } } diff --git a/promql/parse_test.go b/promql/parse_test.go index 07e5ce0c0..28a12a677 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -124,6 +124,20 @@ var testExpr = []struct { }}, }, }, + }, { + input: "1 < bool 2 - 1 * 2", + expected: &BinaryExpr{ + Op: itemLSS, + ReturnBool: true, + LHS: &NumberLiteral{1}, + RHS: &BinaryExpr{ + Op: itemSUB, + LHS: &NumberLiteral{2}, + RHS: &BinaryExpr{ + Op: itemMUL, LHS: &NumberLiteral{1}, RHS: &NumberLiteral{2}, + }, + }, + }, }, { input: "-some_metric", expected: &UnaryExpr{ Op: itemSUB, From 250344b3442efdaafa0aa1d25e343fafb92916a1 Mon Sep 17 00:00:00 2001 From: Patrick Bogen Date: Thu, 3 Mar 2016 09:46:50 -0800 Subject: [PATCH 2/2] use short variable assignment --- promql/parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/promql/parse.go b/promql/parse.go index 91fb6503f..758d87922 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -536,7 +536,7 @@ func (p *parser) expr() Expr { func (p *parser) balance(lhs Expr, op itemType, rhs Expr, vecMatching *VectorMatching, returnBool bool) *BinaryExpr { if lhsBE, ok := lhs.(*BinaryExpr); ok && lhsBE.Op.precedence() < op.precedence() { - var balanced = p.balance(lhsBE.RHS, op, rhs, vecMatching, returnBool) + balanced := p.balance(lhsBE.RHS, op, rhs, vecMatching, returnBool) if lhsBE.Op.isComparisonOperator() && !lhsBE.ReturnBool && balanced.Type() == model.ValScalar && lhsBE.LHS.Type() == model.ValScalar { p.errorf("comparisons between scalars must use BOOL modifier") }