app/vmselect: return metric values from time() cmp_op metric query when cmp_op comparison is true

This aligns MetricsQL behavior to Prometheus' one.

The issue has been identified at https://promlabs.com/promql-compliance-test-results/2020-12-01/victoriametrics/
This commit is contained in:
Aliaksandr Valialkin 2020-12-02 12:08:47 +02:00
parent 7590b8477b
commit 91a4c279cc
2 changed files with 19 additions and 2 deletions

View file

@ -149,11 +149,11 @@ func adjustCmpOps(e metricsql.Expr) metricsql.Expr {
if !metricsql.IsBinaryOpCmp(be.Op) { if !metricsql.IsBinaryOpCmp(be.Op) {
return return
} }
if _, ok := be.Left.(*metricsql.NumberExpr); !ok { if isNumberExpr(be.Right) || !isScalarExpr(be.Left) {
return return
} }
// Convert 'num cmpOp query' expression to `query reverseCmpOp num` expression // Convert 'num cmpOp query' expression to `query reverseCmpOp num` expression
// like Prometheus does. For isntance, `0.5 < foo` must be converted to `foo > 0.5` // like Prometheus does. For instance, `0.5 < foo` must be converted to `foo > 0.5`
// in order to return valid values for `foo` that are bigger than 0.5. // in order to return valid values for `foo` that are bigger than 0.5.
be.Right, be.Left = be.Left, be.Right be.Right, be.Left = be.Left, be.Right
be.Op = getReverseCmpOp(be.Op) be.Op = getReverseCmpOp(be.Op)
@ -161,6 +161,22 @@ func adjustCmpOps(e metricsql.Expr) metricsql.Expr {
return e return e
} }
func isNumberExpr(e metricsql.Expr) bool {
_, ok := e.(*metricsql.NumberExpr)
return ok
}
func isScalarExpr(e metricsql.Expr) bool {
if isNumberExpr(e) {
return true
}
if fe, ok := e.(*metricsql.FuncExpr); ok {
// time() returns scalar in PromQL - see https://prometheus.io/docs/prometheus/latest/querying/functions/#time
return strings.ToLower(fe.Name) == "time"
}
return false
}
func getReverseCmpOp(op string) string { func getReverseCmpOp(op string) string {
switch op { switch op {
case ">": case ">":

View file

@ -7,6 +7,7 @@
* BUGFIX: return `nan` for `a >bool b` query when `a` equals to `nan` like Prometheus does. Previously `0` was returned in this case. This applies to any comparison operation * BUGFIX: return `nan` for `a >bool b` query when `a` equals to `nan` like Prometheus does. Previously `0` was returned in this case. This applies to any comparison operation
with `bool` modifier. See [these docs](https://prometheus.io/docs/prometheus/latest/querying/operators/#comparison-binary-operators) for details. with `bool` modifier. See [these docs](https://prometheus.io/docs/prometheus/latest/querying/operators/#comparison-binary-operators) for details.
* BUGFIX: properly parse hex numbers in MetricsQL. Previously hex numbers with non-decimal digits such as `0x3b` couldn't be parsed. * BUGFIX: properly parse hex numbers in MetricsQL. Previously hex numbers with non-decimal digits such as `0x3b` couldn't be parsed.
* BUGFIX: Handle `time() cmp_op metric` like Prometheus does - i.e. return `metric` value if `cmp_op` comparison is true. Previously `time()` value was returned.
# [v1.48.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.48.0) # [v1.48.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.48.0)