Arithmetic operator precedence doesn't work as documented

[korfuri]

Consider the following expressions:

(1) 1 < 2 evaluates to 1
(2) 1 < 2 - 1 evaluates to 0
(3) 1 < 2 - 1 * 2 evaluates to 1
(4) 1 < 2 - (1 * 2) evaluates to 0

Operator precedence (http://prometheus.io/docs/querying/operators/#binary-operator-precedence) suggests (3) and (4) should both evaluate to 0. However (3) doesn't, and I don't understand why. I've solved it in my rules by parenthesizing more, but the behavior is certainly surprising.

I'm running Prometheus version:
branch debian/sid
date 20150806-01:11:22
go_version go1.4.2
revision 0.14.0+ds-1
version 0.14.0+ds

[beorn7]

A question for Mr Parser AKA @fabxc . :)

[pdbogen]

Here's a test that demonstrates the issue:

diff --git a/promql/parse_test.go b/promql/parse_test.go
index 07e5ce0..763da9a 100644
--- a/promql/parse_test.go
+++ b/promql/parse_test.go
@@ -125,6 +125,19 @@ 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,
                        Expr: &VectorSelector{

Result:

    parse_test.go:1167: error on input '1 < bool 2 - 1 * 2'
    parse_test.go:1168: no match

        expected:
         |---- BinaryExpr :: 1 < BOOL 2 - 1 * 2
         · · · |---- NumberLiteral :: 1
         · · · |---- BinaryExpr :: 2 - 1 * 2
         · · · · · · |---- NumberLiteral :: 2
         · · · · · · |---- BinaryExpr :: 1 * 2
         · · · · · · · · · |---- NumberLiteral :: 1
         · · · · · · · · · |---- NumberLiteral :: 2

        got: 
         |---- BinaryExpr :: 1 < 2 - 1 * 2
         · · · |---- NumberLiteral :: 1
         · · · |---- BinaryExpr :: 2 - 1 * 2
         · · · · · · |---- BinaryExpr :: 2 - 1
         · · · · · · · · · |---- NumberLiteral :: 2
         · · · · · · · · · |---- NumberLiteral :: 1
         · · · · · · |---- NumberLiteral :: 2

...this is indeed parsed wrong. It's parsed as (1) < ((2-1) * 2).

I believe the problem is that the "rebalance" step is not recursive, and it needs to be. Currently for L op R, where L itself is L_L L_op L_R, if L_op precedence is less than op precedence, then the expression is rewritten from L_L L_op L_R op R to L_L L_op [L_r op R]. Hopefully that's not too confusing. The problem occurs when L_R is itself an expression; LR_L LR_op LR_R; this expression is not checked for precedence and it should be.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.