Merge pull request #1451 from pdbogen/origin/1446

rewrite operator balancing to be recursive

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 = p.balance(expr, op, rhs, vecMatching, returnBool)
-			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")
-			}
-		}
 
+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() {
+		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,
+		}
 	}
 }
 

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,