From 90cf356ea13ca337ba694ca4ab938c33f4a21e6f Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin <valyala@gmail.com> Date: Fri, 31 Jan 2020 18:52:30 +0200 Subject: [PATCH] app/vmselect/promql: adjust `and` and `unless` binary operator handling to be consistent with Prometheus --- app/vmselect/promql/binary_op.go | 46 +++++++++++++++++++++++++++----- app/vmselect/promql/exec_test.go | 22 +++++++++++++++ 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/app/vmselect/promql/binary_op.go b/app/vmselect/promql/binary_op.go index fe4c39fabf..e4ecc658aa 100644 --- a/app/vmselect/promql/binary_op.go +++ b/app/vmselect/promql/binary_op.go @@ -32,7 +32,7 @@ var binaryOpFuncs = map[string]binaryOpFunc{ "or": binaryOpOr, "unless": binaryOpUnless, - // New op + // New ops "if": newBinaryOpArithFunc(binaryop.If), "ifnot": newBinaryOpArithFunc(binaryop.Ifnot), "default": newBinaryOpArithFunc(binaryop.Default), @@ -285,10 +285,21 @@ func resetMetricGroupIfRequired(be *metricsql.BinaryOpExpr, ts *timeseries) { func binaryOpAnd(bfa *binaryOpFuncArg) ([]*timeseries, error) { mLeft, mRight := createTimeseriesMapByTagSet(bfa.be, bfa.left, bfa.right) var rvs []*timeseries - for k := range mRight { - if tss := mLeft[k]; tss != nil { - rvs = append(rvs, tss...) + for k, tssRight := range mRight { + tssLeft := mLeft[k] + if tssLeft == nil { + continue } + for i := range tssLeft[0].Values { + if !isAllNaNs(tssRight, i) { + continue + } + for _, tsLeft := range tssLeft { + tsLeft.Values[i] = nan + } + } + tssLeft = removeNaNs(tssLeft) + rvs = append(rvs, tssLeft...) } return rvs, nil } @@ -310,14 +321,35 @@ func binaryOpOr(bfa *binaryOpFuncArg) ([]*timeseries, error) { func binaryOpUnless(bfa *binaryOpFuncArg) ([]*timeseries, error) { mLeft, mRight := createTimeseriesMapByTagSet(bfa.be, bfa.left, bfa.right) var rvs []*timeseries - for k, tss := range mLeft { - if mRight[k] == nil { - rvs = append(rvs, tss...) + for k, tssLeft := range mLeft { + tssRight := mRight[k] + if tssRight == nil { + rvs = append(rvs, tssLeft...) + continue } + for i := range tssLeft[0].Values { + if isAllNaNs(tssRight, i) { + continue + } + for _, tsLeft := range tssLeft { + tsLeft.Values[i] = nan + } + } + tssLeft = removeNaNs(tssLeft) + rvs = append(rvs, tssLeft...) } return rvs, nil } +func isAllNaNs(tss []*timeseries, idx int) bool { + for _, ts := range tss { + if !math.IsNaN(ts.Values[idx]) { + return false + } + } + return true +} + func createTimeseriesMapByTagSet(be *metricsql.BinaryOpExpr, left, right []*timeseries) (map[string][]*timeseries, map[string][]*timeseries) { groupTags := be.GroupModifier.Args groupOp := strings.ToLower(be.GroupModifier.Op) diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index 6b8019acc1..ea793e2618 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -1720,12 +1720,34 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r} f(q, resultExpected) }) + t.Run(`time() and time() > 1300`, func(t *testing.T) { + t.Parallel() + q := `time() and time() > 1300` + r := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{nan, nan, 1400, 1600, 1800, 2000}, + Timestamps: timestampsExpected, + } + resultExpected := []netstorage.Result{r} + f(q, resultExpected) + }) t.Run(`time() unless 2`, func(t *testing.T) { t.Parallel() q := `time() unless 2` resultExpected := []netstorage.Result{} f(q, resultExpected) }) + t.Run(`time() unless time() > 1500`, func(t *testing.T) { + t.Parallel() + q := `time() unless time() > 1500` + r := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{1000, 1200, 1400, nan, nan, nan}, + Timestamps: timestampsExpected, + } + resultExpected := []netstorage.Result{r} + f(q, resultExpected) + }) t.Run(`timseries-with-tags unless 2`, func(t *testing.T) { t.Parallel() q := `label_set(time(), "foo", "bar") unless 2`