diff --git a/promql/lex.go b/promql/lex.go index 52957b2d93dd5a83bef7cec6e0f5e5a606762470..c2c850f3ea031a3c29361c4b26eef5d7116402dc 100644 --- a/promql/lex.go +++ b/promql/lex.go @@ -60,6 +60,17 @@ func (i itemType) isAggregator() bool { return i > aggregatorsStart && i < aggre // Returns false otherwise. func (i itemType) isKeyword() bool { return i > keywordsStart && i < keywordsEnd } +// isCompairsonOperator returns true if the item corresponds to a comparison operator. +// Returns false otherwise. +func (i itemType) isComparisonOperator() bool { + switch i { + case itemEQL, itemNEQ, itemLTE, itemLSS, itemGTE, itemGTR: + return true + default: + return false + } +} + // Constants for operator precedence in expressions. // const LowestPrec = 0 // Non-operators. diff --git a/promql/parse.go b/promql/parse.go index 6903c3acc49ecb507b07bd7fa5e7ebb54b616db8..c15ee6aed867714a431c45c888bb6edc51619e14 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -488,10 +488,7 @@ func (p *parser) expr() Expr { returnBool := false // Parse bool modifier. if p.peek().typ == itemBool { - switch op { - case itemEQL, itemNEQ, itemLTE, itemLSS, itemGTE, itemGTR: - break - default: + if !op.isComparisonOperator() { p.errorf("bool modifier can only be used on comparison operators") } p.next() @@ -540,6 +537,9 @@ func (p *parser) expr() Expr { }, 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, @@ -548,7 +548,11 @@ func (p *parser) expr() Expr { 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") + } } + } } diff --git a/promql/parse_test.go b/promql/parse_test.go index ecbc95da871b435d5a59935f9898c519c8cb3d51..ab9b78d03b5c2f57f4a679bb41a6dac5c99e945b 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -85,23 +85,20 @@ var testExpr = []struct { input: "1 / 1", expected: &BinaryExpr{itemDIV, &NumberLiteral{1}, &NumberLiteral{1}, nil, false}, }, { - input: "1 == 1", - expected: &BinaryExpr{itemEQL, &NumberLiteral{1}, &NumberLiteral{1}, nil, false}, + input: "1 == bool 1", + expected: &BinaryExpr{itemEQL, &NumberLiteral{1}, &NumberLiteral{1}, nil, true}, }, { - input: "1 != 1", - expected: &BinaryExpr{itemNEQ, &NumberLiteral{1}, &NumberLiteral{1}, nil, false}, + input: "1 != bool 1", + expected: &BinaryExpr{itemNEQ, &NumberLiteral{1}, &NumberLiteral{1}, nil, true}, }, { - input: "1 > 1", - expected: &BinaryExpr{itemGTR, &NumberLiteral{1}, &NumberLiteral{1}, nil, false}, + input: "1 > bool 1", + expected: &BinaryExpr{itemGTR, &NumberLiteral{1}, &NumberLiteral{1}, nil, true}, }, { - input: "1 >= 1", - expected: &BinaryExpr{itemGTE, &NumberLiteral{1}, &NumberLiteral{1}, nil, false}, + input: "1 >= bool 1", + expected: &BinaryExpr{itemGTE, &NumberLiteral{1}, &NumberLiteral{1}, nil, true}, }, { - input: "1 < 1", - expected: &BinaryExpr{itemLSS, &NumberLiteral{1}, &NumberLiteral{1}, nil, false}, - }, { - input: "1 <= 1", - expected: &BinaryExpr{itemLTE, &NumberLiteral{1}, &NumberLiteral{1}, nil, false}, + input: "1 < bool 1", + expected: &BinaryExpr{itemLSS, &NumberLiteral{1}, &NumberLiteral{1}, nil, true}, }, { input: "1 <= bool 1", expected: &BinaryExpr{itemLTE, &NumberLiteral{1}, &NumberLiteral{1}, nil, true}, @@ -203,6 +200,10 @@ var testExpr = []struct { input: "1 and 1", fail: true, errMsg: "AND and OR not allowed in binary scalar expression", + }, { + input: "1 == 1", + fail: true, + errMsg: "parse error at char 7: comparisons between scalars must use BOOL modifier", }, { input: "1 or 1", fail: true, diff --git a/promql/testdata/comparison.test b/promql/testdata/comparison.test index 4ce49daa74231e5041e0c1e20382959bb1ebf4ec..012d5891ab701798d9ceee9efb19777747d95d49 100644 --- a/promql/testdata/comparison.test +++ b/promql/testdata/comparison.test @@ -40,12 +40,6 @@ eval instant at 50m SUM(http_requests) BY (job) != bool SUM(http_requests) BY (j {job="app-server"} 0 -eval instant at 50m 0 == 1 - 0 - -eval instant at 50m 1 == 1 - 1 - eval instant at 50m 0 == bool 1 0