diff --git a/scripts/comparator/basic_queries/queries.json b/scripts/comparator/basic_queries/queries.json index dd685b99a9..0f1dc99e26 100644 --- a/scripts/comparator/basic_queries/queries.json +++ b/scripts/comparator/basic_queries/queries.json @@ -1,36 +1,36 @@ [ { - "queryGroup":"scalar", - "queries":[ + "queryGroup": "scalar", + "queries": [ "42", "time()" ], - "steps" : [ + "steps": [ "1m" ] }, { - "queryGroup":"fetch", - "queries":[ + "queryGroup": "fetch", + "queries": [ "quail", "quail offset 60s" ], - "steps" : [ + "steps": [ "15s", "30s", "1m" ] }, { - "queryGroup":"temporal", - "queries":[ + "queryGroup": "temporal", + "queries": [ "rate(multi_1[1m])", "irate(multi_1[5m])", "delta(multi_1[123s])", "idelta(multi_1[1m] offset 1h)", "deriv(multi_1[3m])" ], - "steps" : [ + "steps": [ "15s", "30s", "1m", @@ -38,69 +38,80 @@ ] }, { - "queryGroup":"binary", - "queries":[ + "queryGroup": "binary", + "queries": [ "quail*1", "1-quail", "quail*quail", "quail offset 1m / quail" ], - "steps" : [ + "steps": [ "15s", "30s", "1m" ] }, { - "queryGroup":"aggregation", - "queries":[ + "queryGroup": "aggregation", + "queries": [ "sum(foobar{foobar=\"qux\"})", "sum(foobar{foobar=\"qux\"}) - 1", "sum(foobar{foobar=\"qux\"} offset 1m)" ], - "steps" : [ + "steps": [ "15s", "30s", "1m" ] }, { - "queryGroup":"transform", - "queries":[ + "queryGroup": "transform", + "queries": [ "clamp_max(quail, 0.5)", "clamp_min(quail offset 60s, 0.5)", "sum(foobar{foobar=\"qux\"}) - 1", "sum(foobar{foobar=\"qux\"} offset 1m)" ], - "steps" : [ + "steps": [ "15s", "30s", "1m" ] }, { - "queryGroup":"label", - "queries":[ + "queryGroup": "label", + "queries": [ "label_replace(quail,\"foo\", \"$1!\", \"name\", \"(.*)\")", "label_replace(quail offset 1m,\"foo\", \"$1!\", \"name\", \"(.*)\")", "label_replace(quail,\"foo\", \"$1!\", \"name\", \"(.*)\")-100", "label_join(quail,\"quince\", \"!\", \"foobar\", \"name\")" ], - "steps" : [ + "steps": [ "15s", "30s", "1m" ] }, { - "queryGroup":"topk", + "queryGroup": "topk", "reruns": 5, - "queries":[ + "queries": [ "topk(2, quack)", "topk(2, avg_over_time(quack[30s]))", "topk(2, avg(avg_over_time(quack[30s])) by (name))" ], - "steps" : [ + "steps": [ + "1m" + ] + }, + { + "queryGroup": "quantile", + "queries": [ + "quantile(0, multi_10)", + "quantile(0.8, avg_over_time(multi_10[30s]))", + "quantile(1, avg(avg_over_time(multi_10[30s])) by (name))" + ], + "steps": [ "1m" ] } diff --git a/src/query/functions/aggregation/quantile.go b/src/query/functions/aggregation/quantile.go index d28295a4e1..2a5331a123 100644 --- a/src/query/functions/aggregation/quantile.go +++ b/src/query/functions/aggregation/quantile.go @@ -44,7 +44,7 @@ func makeQuantileFn(opType string, q float64) (aggregationFn, bool) { } func bucketedQuantileFn(q float64, values []float64, bucket []int) float64 { - if len(bucket) == 0 || len(values) == 0 { + if math.IsNaN(q) || len(bucket) == 0 || len(values) == 0 { return math.NaN() } diff --git a/src/query/functions/aggregation/quantile_test.go b/src/query/functions/aggregation/quantile_test.go index a6f9be8ef4..7db255e728 100644 --- a/src/query/functions/aggregation/quantile_test.go +++ b/src/query/functions/aggregation/quantile_test.go @@ -117,6 +117,46 @@ func TestQuantileFnSingleNonNan(t *testing.T) { test.EqualsWithNansWithDelta(t, expected, actual, math.Pow10(-5)) } +func TestQuantileNanAndEmptyArguments(t *testing.T) { + tests := []struct { + name string + q float64 + values []float64 + bucket []int + }{ + { + name: "q = NaN", + q: math.NaN(), + values: []float64{0.0, 1.0}, + bucket: []int{0, 1}, + }, + { + name: "empty values and bucket", + q: 0.8, + values: []float64{}, + bucket: []int{}, + }, + { + name: "empty bucket", + q: 0.8, + values: []float64{0.0, 1.0}, + bucket: []int{}, + }, + { + name: "empty values", + q: 0.8, + values: []float64{}, + bucket: []int{0, 1}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + results := bucketedQuantileFn(tt.q, tt.values, tt.bucket) + assert.True(t, math.IsNaN(results)) + }) + } +} + func TestQuantileCreationFn(t *testing.T) { n := 0.145 op, success := makeQuantileFn("badOp", n) diff --git a/src/query/parser/promql/matchers.go b/src/query/parser/promql/matchers.go index af1237cbd3..a0d99e9262 100644 --- a/src/query/parser/promql/matchers.go +++ b/src/query/parser/promql/matchers.go @@ -91,11 +91,11 @@ func NewAggregationOperator(expr *promql.AggregateExpr) (parser.Params, error) { } op := getAggOpType(opType) - if op == common.UnknownOpType { + switch op { + case common.UnknownOpType: return nil, fmt.Errorf("operator not supported: %s", opType) - } - if op == aggregation.BottomKType || op == aggregation.TopKType { + case aggregation.BottomKType, aggregation.TopKType: val, err := resolveScalarArgument(expr.Param) if err != nil { return nil, err @@ -103,13 +103,19 @@ func NewAggregationOperator(expr *promql.AggregateExpr) (parser.Params, error) { nodeInformation.Parameter = val return aggregation.NewTakeOp(op, nodeInformation) - } - if op == aggregation.CountValuesType { + case aggregation.CountValuesType: nodeInformation.StringParameter = expr.Param.String() return aggregation.NewCountValuesOp(op, nodeInformation) - } + case aggregation.QuantileType: + val, err := resolveScalarArgument(expr.Param) + if err != nil { + return nil, err + } + + nodeInformation.Parameter = val + } return aggregation.NewAggregationOp(op, nodeInformation) } diff --git a/src/query/test/compatibility/testdata/aggregators.test b/src/query/test/compatibility/testdata/aggregators.test index 716395e8b7..854c022e15 100644 --- a/src/query/test/compatibility/testdata/aggregators.test +++ b/src/query/test/compatibility/testdata/aggregators.test @@ -286,13 +286,12 @@ load 10s data{test="uneven samples",point="c"} 4 foo .8 -# FAILING issue #8 (quantile) -#eval instant at 1m quantile without(point)(0.8, data) -# {test="two samples"} 0.8 -# {test="three samples"} 1.6 -# {test="uneven samples"} 2.8 +eval instant at 1m quantile without(point)(0.8, data) + {test="two samples"} 0.8 + {test="three samples"} 1.6 + {test="uneven samples"} 2.8 -# Bug #5276. +# FAILING issue #56. scalar() as a quantile()/topk()/bottomk() argument produces NaN #eval instant at 1m quantile without(point)(scalar(foo), data) # {test="two samples"} 0.8 # {test="three samples"} 1.6