Allow reversing vector and scalar arguments in binops.

This allows putting a scalar as the first argument of a binary operator in which the second argument is a vector: <scalar> <binop> <vector> For example, 1 / http_requests_total ...will output a vector in which every sample value is 1 divided by the respective input vector element. This even works for filter binary operators now: 1 == http_requests_total Returns a vector with all values set to 1 for every element in http_requests_total whose initial value was 1. Note: For filter binary operators, the resulting values are always taken from the left-hand-side of the operation, no matter whether the scalar or the vector argument is the left-hand-side. That is, 1 != http_requests_total ...will set all result vector sample values to 1, although these are exactly the sample elements that were != 1 in the input vector. If you want to just filter elements without changing their sample values, you still need to do: http_requests_total != 1 The new filter form is a bit exotic, and so probably won't be used often. But it was easier to implement it than disallow it completely or change its behavior. Change-Id: Idd083f2bd3a1219ba1560cf4ace42f5b82e797a5
synweap15 · Apr 8, 2014 · d411a7d · d411a7d
1 parent c7c0b33
commit d411a7d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 14 deletions.
diff --git a/rules/ast/ast.go b/rules/ast/ast.go
@@ -195,11 +195,12 @@ type (
 		vector          VectorNode
 	}
 
-	// VectorArithExpr represents an arithmetic expression of
-	// vector type.
+	// VectorArithExpr represents an arithmetic expression of vector type. At
+	// least one of the two operand Nodes must be a VectorNode. The other may be
+	// a VectorNode or ScalarNode. Both criteria are checked at runtime.
 	VectorArithExpr struct {
 		opType BinOpType
-		lhs    VectorNode
+		lhs    Node
 		rhs    Node
 	}
 )
@@ -639,9 +640,20 @@ func labelsEqual(labels1, labels2 clientmodel.Metric) bool {
 // Eval implements the VectorNode interface and returns the result of
 // the expression.
 func (node *VectorArithExpr) Eval(timestamp clientmodel.Timestamp, view *viewAdapter) Vector {
-	lhs := node.lhs.Eval(timestamp, view)
 	result := Vector{}
-	if node.rhs.Type() == SCALAR {
+	if node.lhs.Type() == SCALAR && node.rhs.Type() == VECTOR {
+		lhs := node.lhs.(ScalarNode).Eval(timestamp, view)
+		rhs := node.rhs.(VectorNode).Eval(timestamp, view)
+		for _, rhsSample := range rhs {
+			value, keep := evalVectorBinop(node.opType, lhs, rhsSample.Value)
+			if keep {
+				rhsSample.Value = value
+				result = append(result, rhsSample)
+			}
+		}
+		return result
+	} else if node.lhs.Type() == VECTOR && node.rhs.Type() == SCALAR {
+		lhs := node.lhs.(VectorNode).Eval(timestamp, view)
 		rhs := node.rhs.(ScalarNode).Eval(timestamp, view)
 		for _, lhsSample := range lhs {
 			value, keep := evalVectorBinop(node.opType, lhsSample.Value, rhs)
@@ -651,7 +663,8 @@ func (node *VectorArithExpr) Eval(timestamp clientmodel.Timestamp, view *viewAda
 			}
 		}
 		return result
-	} else if node.rhs.Type() == VECTOR {
+	} else if node.lhs.Type() == VECTOR && node.rhs.Type() == VECTOR {
+		lhs := node.lhs.(VectorNode).Eval(timestamp, view)
 		rhs := node.rhs.(VectorNode).Eval(timestamp, view)
 		for _, lhsSample := range lhs {
 			for _, rhsSample := range rhs {
@@ -804,9 +817,6 @@ func NewArithExpr(opType BinOpType, lhs Node, rhs Node) (Node, error) {
 	if !nodesHaveTypes(Nodes{lhs, rhs}, []ExprType{SCALAR, VECTOR}) {
 		return nil, errors.New("binary operands must be of vector or scalar type")
 	}
-	if lhs.Type() == SCALAR && rhs.Type() == VECTOR {
-		return nil, errors.New("left side of vector binary operation must be of vector type")
-	}
 
 	if opType == AND || opType == OR {
 		if lhs.Type() == SCALAR || rhs.Type() == SCALAR {
@@ -817,7 +827,7 @@ func NewArithExpr(opType BinOpType, lhs Node, rhs Node) (Node, error) {
 	if lhs.Type() == VECTOR || rhs.Type() == VECTOR {
 		return &VectorArithExpr{
 			opType: opType,
-			lhs:    lhs.(VectorNode),
+			lhs:    lhs,
 			rhs:    rhs,
 		}, nil
 	}

diff --git a/rules/rules_test.go b/rules/rules_test.go
@@ -209,6 +209,22 @@ func TestExpressions(t *testing.T) {
 			},
 			fullRanges:     0,
 			intervalRanges: 8,
+		}, {
+			expr: `2 - SUM(http_requests) BY (job)`,
+			output: []string{
+				`http_requests{job="api-server"} => -998 @[%v]`,
+				`http_requests{job="app-server"} => -2598 @[%v]`,
+			},
+			fullRanges:     0,
+			intervalRanges: 8,
+		}, {
+			expr: `1000 / SUM(http_requests) BY (job)`,
+			output: []string{
+				`http_requests{job="api-server"} => 1 @[%v]`,
+				`http_requests{job="app-server"} => 0.38461538461538464 @[%v]`,
+			},
+			fullRanges:     0,
+			intervalRanges: 8,
 		}, {
 			expr: `SUM(http_requests) BY (job) - 2`,
 			output: []string{
@@ -240,6 +256,13 @@ func TestExpressions(t *testing.T) {
 			},
 			fullRanges:     0,
 			intervalRanges: 8,
+		}, {
+			expr: `1000 < SUM(http_requests) BY (job)`,
+			output: []string{
+				`http_requests{job="app-server"} => 1000 @[%v]`,
+			},
+			fullRanges:     0,
+			intervalRanges: 8,
 		}, {
 			expr: `SUM(http_requests) BY (job) <= 1000`,
 			output: []string{
@@ -398,14 +421,14 @@ func TestExpressions(t *testing.T) {
 			// Empty expressions shouldn"t parse.
 			expr:       ``,
 			shouldFail: true,
-		}, {
-			// Subtracting a vector from a scalar is not supported.
-			expr:       `1 - http_requests`,
-			shouldFail: true,
 		}, {
 			// Interval durations can"t be in quotes.
 			expr:       `http_requests["1m"]`,
 			shouldFail: true,
+		}, {
+			// Binop arguments need to be scalar or vector.
+			expr:       `http_requests - http_requests[1m]`,
+			shouldFail: true,
 		}, {
 			expr: `http_requests{group!="canary"}`,
 			output: []string{