From e087ce4c0af44f5899e5c670511b311913bca721 Mon Sep 17 00:00:00 2001 From: Artem Nikolayevsky Date: Thu, 1 Aug 2019 10:10:32 -0400 Subject: [PATCH 1/6] [query] Fix absent function --- src/query/functions/aggregation/absent.go | 156 ++++++++++++++++++ src/query/functions/aggregation/base.go | 29 ++-- src/query/functions/aggregation/base_test.go | 2 +- src/query/functions/aggregation/function.go | 27 ++- .../functions/aggregation/function_test.go | 20 ++- .../functions/aggregation/quantile_test.go | 2 +- src/query/functions/aggregation/take_test.go | 6 +- src/query/functions/linear/absent.go | 75 --------- src/query/functions/linear/absent_test.go | 70 -------- src/query/parser/promql/parse_test.go | 3 +- src/query/parser/promql/types.go | 4 +- 11 files changed, 213 insertions(+), 181 deletions(-) create mode 100644 src/query/functions/aggregation/absent.go delete mode 100644 src/query/functions/linear/absent.go delete mode 100644 src/query/functions/linear/absent_test.go diff --git a/src/query/functions/aggregation/absent.go b/src/query/functions/aggregation/absent.go new file mode 100644 index 0000000000..ed44ec99bc --- /dev/null +++ b/src/query/functions/aggregation/absent.go @@ -0,0 +1,156 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package aggregation + +import ( + "fmt" + "math" + "time" + + "github.com/m3db/m3/src/query/block" + "github.com/m3db/m3/src/query/executor/transform" + "github.com/m3db/m3/src/query/functions/utils" + "github.com/m3db/m3/src/query/models" + "github.com/m3db/m3/src/query/parser" +) + +const ( + // AbsentType returns 1 if there are no elements in this step, or if no series + // are present in the current block. + AbsentType = "absent" +) + +// NewAbsentOp creates a new absent operation. +func NewAbsentOp() parser.Params { + return newAbsentOp() +} + +// absentOp stores required properties for absent ops. +type absentOp struct{} + +// OpType for the operator. +func (o absentOp) OpType() string { + return AbsentType +} + +// String representation. +func (o absentOp) String() string { + return fmt.Sprintf("type: absent") +} + +// Node creates an execution node. +func (o absentOp) Node( + controller *transform.Controller, + _ transform.Options, +) transform.OpNode { + return &absentNode{ + op: o, + controller: controller, + } +} + +func newAbsentOp() absentOp { + return absentOp{} +} + +// absentNode is different from base node as it uses no grouping and has +// special handling for the 0-series case. +type absentNode struct { + op parser.Params + controller *transform.Controller +} + +func (n *absentNode) Params() parser.Params { + return n.op +} + +// Process the block +func (n *absentNode) Process(queryCtx *models.QueryContext, + ID parser.NodeID, b block.Block) error { + return transform.ProcessSimpleBlock(n, n.controller, queryCtx, ID, b) +} + +func (n *absentNode) ProcessBlock(queryCtx *models.QueryContext, + ID parser.NodeID, b block.Block) (block.Block, error) { + stepIter, err := b.StepIter() + if err != nil { + return nil, err + } + + // Absent should + var ( + meta = stepIter.Meta() + seriesMetas = stepIter.SeriesMeta() + tagOpts = meta.Tags.Opts + ) + + // If no series in the input, return a scalar block with value 1. + if len(seriesMetas) == 0 { + return block.NewScalar( + func(_ time.Time) float64 { return 1 }, + meta.Bounds, + tagOpts, + ), nil + } + + // NB: pull any common tags out into the created series. + dupeTags, _ := utils.DedupeMetadata(seriesMetas, tagOpts) + mergedCommonTags := meta.Tags.Add(dupeTags) + meta.Tags = models.NewTags(0, tagOpts) + emptySeriesMeta := []block.SeriesMeta{ + block.SeriesMeta{ + Tags: mergedCommonTags, + Name: []byte{}, + }, + } + + builder, err := n.controller.BlockBuilder(queryCtx, meta, emptySeriesMeta) + if err != nil { + return nil, err + } + + if err = builder.AddCols(1); err != nil { + return nil, err + } + + for index := 0; stepIter.Next(); index++ { + step := stepIter.Current() + values := step.Values() + + var val float64 = 1 + for _, v := range values { + if !math.IsNaN(v) { + val = 0 + break + } + } + + if err := builder.AppendValue(index, val); err != nil { + return nil, err + } + } + + if err = stepIter.Err(); err != nil { + return nil, err + } + + return builder.Build(), nil +} diff --git a/src/query/functions/aggregation/base.go b/src/query/functions/aggregation/base.go index 9f750f43ed..3d3c0d6966 100644 --- a/src/query/functions/aggregation/base.go +++ b/src/query/functions/aggregation/base.go @@ -33,6 +33,7 @@ import ( type aggregationFn func(values []float64, bucket []int) float64 var aggregationFunctions = map[string]aggregationFn{ + AbsentType: absentFn, SumType: sumFn, MinType: minFn, MaxType: maxFn, @@ -42,20 +43,21 @@ var aggregationFunctions = map[string]aggregationFn{ CountType: countFn, } -// NodeParams contains additional parameters required for aggregation ops +// NodeParams contains additional parameters required for aggregation ops. type NodeParams struct { - // MatchingTags is the set of tags by which the aggregation groups output series + // MatchingTags is the set of tags by which the aggregation groups + // output series. MatchingTags [][]byte - // Without indicates if series should use only the MatchingTags or if MatchingTags - // should be excluded from grouping + // Without indicates if series should use only the MatchingTags or if + // MatchingTags should be excluded from grouping. Without bool - // Parameter is the param value for the aggregation op when appropriate + // Parameter is the param value for the aggregation op when appropriate. Parameter float64 - // StringParameter is the string representation of the param value + // StringParameter is the string representation of the param value. StringParameter string } -// NewAggregationOp creates a new aggregation operation +// NewAggregationOp creates a new aggregation operation. func NewAggregationOp( opType string, params NodeParams, @@ -71,25 +73,28 @@ func NewAggregationOp( return baseOp{}, fmt.Errorf("operator not supported: %s", opType) } -// baseOp stores required properties for the baseOp +// baseOp stores required properties for the baseOp. type baseOp struct { params NodeParams opType string aggFn aggregationFn } -// OpType for the operator +// OpType for the operator. func (o baseOp) OpType() string { return o.opType } -// String representation +// String representation. func (o baseOp) String() string { return fmt.Sprintf("type: %s", o.OpType()) } -// Node creates an execution node -func (o baseOp) Node(controller *transform.Controller, _ transform.Options) transform.OpNode { +// Node creates an execution node. +func (o baseOp) Node( + controller *transform.Controller, + _ transform.Options, +) transform.OpNode { return &baseNode{ op: o, controller: controller, diff --git a/src/query/functions/aggregation/base_test.go b/src/query/functions/aggregation/base_test.go index 38642649f8..91bbb453cd 100644 --- a/src/query/functions/aggregation/base_test.go +++ b/src/query/functions/aggregation/base_test.go @@ -108,7 +108,7 @@ func TestFunctionFilteringWithoutA(t *testing.T) { expected := [][]float64{ // stddev of first two series {0, 0, 2.5, 2.5, 2.5}, - // stddev of third, fourth, and fifth series + // stddev of third, fourth, and fifth series {36.81787, 77.17225, 118.97712, 161.10728, 203.36065}, // stddev of sixth series {0, 0, 0, 0, 0}, diff --git a/src/query/functions/aggregation/function.go b/src/query/functions/aggregation/function.go index 0243695d04..5d15019d36 100644 --- a/src/query/functions/aggregation/function.go +++ b/src/query/functions/aggregation/function.go @@ -25,24 +25,35 @@ import ( ) const ( - // SumType adds all non nan elements in a list of series + // SumType adds all non nan elements in a list of series. SumType = "sum" - // MinType takes the minimum all non nan elements in a list of series + // MinType takes the minimum all non nan elements in a list of series. MinType = "min" - // MaxType takes the maximum all non nan elements in a list of series + // MaxType takes the maximum all non nan elements in a list of series. MaxType = "max" - // AverageType averages all non nan elements in a list of series + // AverageType averages all non nan elements in a list of series. AverageType = "avg" // StandardDeviationType takes the population standard deviation of all non - // nan elements in a list of series + // nan elements in a list of series. StandardDeviationType = "stddev" // StandardVarianceType takes the population standard variance of all non - // nan elements in a list of series + // nan elements in a list of series. StandardVarianceType = "var" - // CountType counts all non nan elements in a list of series + // CountType counts all non nan elements in a list of series. CountType = "count" ) +func absentFn(values []float64, bucket []int) float64 { + for _, idx := range bucket { + v := values[idx] + if !math.IsNaN(v) { + return math.NaN() + } + } + + return 1 +} + func sumAndCount(values []float64, bucket []int) (float64, float64) { sum := 0.0 count := 0.0 @@ -54,7 +65,7 @@ func sumAndCount(values []float64, bucket []int) (float64, float64) { } } - // If all elements are NaN, sum should be NaN + // If all elements are NaN, sum should be NaN. if count == 0 { sum = math.NaN() } diff --git a/src/query/functions/aggregation/function_test.go b/src/query/functions/aggregation/function_test.go index 6e9e8ad452..799a29181e 100644 --- a/src/query/functions/aggregation/function_test.go +++ b/src/query/functions/aggregation/function_test.go @@ -33,6 +33,8 @@ type funcTest struct { expected []float64 } +var nan = math.NaN() + var fnTest = []struct { name string values []float64 @@ -110,7 +112,7 @@ var fnTest = []struct { }, { "many values, one index, with nans", - []float64{10, math.NaN(), 10, math.NaN(), 8, 4}, + []float64{10, nan, 10, nan, 8, 4}, [][]int{{0, 1, 2, 3, 4, 5}}, []funcTest{ {SumType, sumFn, []float64{32}}, {MinType, minFn, []float64{4}}, @@ -119,19 +121,21 @@ var fnTest = []struct { {StandardDeviationType, stddevFn, []float64{2.44949}}, {StandardVarianceType, varianceFn, []float64{6}}, {CountType, countFn, []float64{4}}, + {AbsentType, absentFn, []float64{nan}}, }, }, { "only nans", - []float64{math.NaN(), math.NaN(), math.NaN(), math.NaN()}, + []float64{nan, nan, nan, nan}, [][]int{{0, 1, 2, 3}}, []funcTest{ - {SumType, sumFn, []float64{math.NaN()}}, - {MinType, minFn, []float64{math.NaN()}}, - {MaxType, maxFn, []float64{math.NaN()}}, - {AverageType, averageFn, []float64{math.NaN()}}, - {StandardDeviationType, stddevFn, []float64{math.NaN()}}, - {StandardVarianceType, varianceFn, []float64{math.NaN()}}, + {SumType, sumFn, []float64{nan}}, + {MinType, minFn, []float64{nan}}, + {MaxType, maxFn, []float64{nan}}, + {AverageType, averageFn, []float64{nan}}, + {StandardDeviationType, stddevFn, []float64{nan}}, + {StandardVarianceType, varianceFn, []float64{nan}}, {CountType, countFn, []float64{0}}, + {AbsentType, absentFn, []float64{1}}, }, }, { diff --git a/src/query/functions/aggregation/quantile_test.go b/src/query/functions/aggregation/quantile_test.go index 85cddce125..2943596c4e 100644 --- a/src/query/functions/aggregation/quantile_test.go +++ b/src/query/functions/aggregation/quantile_test.go @@ -144,7 +144,7 @@ func TestQuantileFunctionFilteringWithoutA(t *testing.T) { expected := [][]float64{ // 0.6 quantile of first two series {0, 6, 5, 6, 7}, - // 0.6 quantile of third, fourth, and fifth series + // 0.6 quantile of third, fourth, and fifth series {60, 88, 116, 144, 172}, // stddev of sixth series {600, 700, 800, 900, 1000}, diff --git a/src/query/functions/aggregation/take_test.go b/src/query/functions/aggregation/take_test.go index ddad9ff8d3..c0a5bef13b 100644 --- a/src/query/functions/aggregation/take_test.go +++ b/src/query/functions/aggregation/take_test.go @@ -78,7 +78,7 @@ func TestTakeBottomFunctionFilteringWithoutA(t *testing.T) { // Taking bottomk(1) of first two series, keeping both series {0, math.NaN(), 2, 3, 4}, {math.NaN(), 6, math.NaN(), math.NaN(), math.NaN()}, - // Taking bottomk(1) of third, fourth, and fifth two series, keeping all series + // Taking bottomk(1) of third, fourth, and fifth two series, keeping all series {10, 20, 30, 40, 50}, {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, @@ -102,7 +102,7 @@ func TestTakeTopFunctionFilteringWithoutA(t *testing.T) { // Taking bottomk(1) of first two series, keeping both series {0, math.NaN(), math.NaN(), math.NaN(), math.NaN()}, {math.NaN(), 6, 7, 8, 9}, - // Taking bottomk(1) of third, fourth, and fifth two series, keeping all series + // Taking bottomk(1) of third, fourth, and fifth two series, keeping all series {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, {100, 200, 300, 400, 500}, @@ -126,7 +126,7 @@ func TestTakeTopFunctionFilteringWithoutALessThanOne(t *testing.T) { // Taking bottomk(1) of first two series, keeping both series {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, - // Taking bottomk(1) of third, fourth, and fifth two series, keeping all series + // Taking bottomk(1) of third, fourth, and fifth two series, keeping all series {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, {math.NaN(), math.NaN(), math.NaN(), math.NaN(), math.NaN()}, diff --git a/src/query/functions/linear/absent.go b/src/query/functions/linear/absent.go deleted file mode 100644 index 81c17fa124..0000000000 --- a/src/query/functions/linear/absent.go +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) 2018 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -package linear - -import ( - "math" - - "github.com/m3db/m3/src/query/executor/transform" -) - -// FIXME: This is incorrect functionality. -// Tracking issue https://github.com/m3db/m3/issues/1847 -//' This should be an aggregation function that works in a step-wise fashion. -// AbsentType returns a timeseries with all NaNs if the timeseries passed in has any non NaNs, -// and returns a timeseries with the value 1 if the timeseries passed in has no elements -const AbsentType = "absent" - -// NewAbsentOp creates a new base linear transform with an absent node -func NewAbsentOp() BaseOp { - return BaseOp{ - operatorType: AbsentType, - processorFn: newAbsentNode, - } -} - -func newAbsentNode(op BaseOp, controller *transform.Controller) Processor { - return &absentNode{ - op: op, - controller: controller, - } -} - -type absentNode struct { - op BaseOp - controller *transform.Controller -} - -func (c *absentNode) Process(values []float64) []float64 { - num := 1.0 - if !allNaNs(values) { - num = math.NaN() - } - - for i := range values { - values[i] = num - } - return values -} - -func allNaNs(vals []float64) bool { - for _, i := range vals { - if !math.IsNaN(i) { - return false - } - } - return true -} diff --git a/src/query/functions/linear/absent_test.go b/src/query/functions/linear/absent_test.go deleted file mode 100644 index 530087d237..0000000000 --- a/src/query/functions/linear/absent_test.go +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright (c) 2018 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -package linear - -import ( - "math" - "testing" - - "github.com/m3db/m3/src/query/executor/transform" - "github.com/m3db/m3/src/query/models" - "github.com/m3db/m3/src/query/parser" - "github.com/m3db/m3/src/query/test" - "github.com/m3db/m3/src/query/test/executor" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -var ( - nan = math.NaN() -) - -func TestAbsentWithValues(t *testing.T) { - values, bounds := test.GenerateValuesAndBounds(nil, nil) - block := test.NewBlockFromValues(bounds, values) - c, sink := executor.NewControllerWithSink(parser.NodeID(1)) - node := NewAbsentOp().Node(c, transform.Options{}) - err := node.Process(models.NoopQueryContext(), parser.NodeID(0), block) - require.NoError(t, err) - assert.Len(t, sink.Values, 2) - expected := [][]float64{ - {nan, nan, nan, nan, nan}, - {nan, nan, nan, nan, nan}, - } - test.EqualsWithNans(t, expected, sink.Values) -} - -func TestAbsentWithNoValues(t *testing.T) { - v := [][]float64{ - {nan, nan, nan, nan, nan}, - {nan, nan, nan, nan, nan}, - } - - values, bounds := test.GenerateValuesAndBounds(v, nil) - block := test.NewBlockFromValues(bounds, values) - c, sink := executor.NewControllerWithSink(parser.NodeID(1)) - node := NewAbsentOp().Node(c, transform.Options{}) - err := node.Process(models.NoopQueryContext(), parser.NodeID(0), block) - require.NoError(t, err) - assert.Len(t, sink.Values, 2) - assert.Equal(t, [][]float64{{1, 1, 1, 1, 1}, {1, 1, 1, 1, 1}}, sink.Values) -} diff --git a/src/query/parser/promql/parse_test.go b/src/query/parser/promql/parse_test.go index 9b82d6263f..9ba5af5c53 100644 --- a/src/query/parser/promql/parse_test.go +++ b/src/query/parser/promql/parse_test.go @@ -154,6 +154,8 @@ var aggregateParseTests = []struct { {"bottomk(3, up)", aggregation.BottomKType}, {"quantile(3, up)", aggregation.QuantileType}, {"count_values(\"some_name\", up)", aggregation.CountValuesType}, + + {"absent(up)", aggregation.AbsentType}, } func TestAggregateParses(t *testing.T) { @@ -181,7 +183,6 @@ var linearParseTests = []struct { expectedType string }{ {"abs(up)", linear.AbsType}, - {"absent(up)", linear.AbsentType}, {"ceil(up)", linear.CeilType}, {"clamp_min(up, 1)", linear.ClampMinType}, {"clamp_max(up, 1)", linear.ClampMaxType}, diff --git a/src/query/parser/promql/types.go b/src/query/parser/promql/types.go index dc3c750f20..dcbb1ff192 100644 --- a/src/query/parser/promql/types.go +++ b/src/query/parser/promql/types.go @@ -209,8 +209,8 @@ func NewFunctionExpr( p, err = linear.NewMathOp(name) return p, true, err - case linear.AbsentType: - p = linear.NewAbsentOp() + case aggregation.AbsentType: + p = aggregation.NewAbsentOp() return p, true, err case linear.ClampMinType, linear.ClampMaxType: From d3aadafb57bba18710614b7405f55dbd0e9c2ff7 Mon Sep 17 00:00:00 2001 From: Artem Nikolayevsky Date: Sat, 10 Aug 2019 09:25:09 -0700 Subject: [PATCH 2/6] [query] Fix absent function --- src/query/block/block_test.go | 80 +++++++++ src/query/block/column.go | 4 +- src/query/block/empty.go | 130 ++++++++++++++ src/query/block/empty_test.go | 103 +++++++++++ src/query/block/meta.go | 45 +++++ src/query/block/meta_test.go | 37 ++++ src/query/block/scalar.go | 21 +-- src/query/block/scalar_test.go | 27 ++- src/query/block/types.go | 12 -- src/query/executor/transform/exec_test.go | 14 +- src/query/functions/aggregation/absent.go | 70 +++++--- .../functions/aggregation/absent_test.go | 160 ++++++++++++++++++ src/query/functions/binary/binary.go | 3 +- src/query/functions/binary/binary_test.go | 48 ++++-- src/query/functions/linear/base.go | 123 -------------- src/query/functions/linear/clamp.go | 67 +++----- src/query/functions/linear/clamp_test.go | 56 +++++- .../functions/linear/histogram_quantile.go | 6 +- src/query/functions/linear/math.go | 2 +- src/query/functions/linear/round_test.go | 49 +++--- src/query/functions/scalar/base.go | 9 +- src/query/models/bounds.go | 24 +-- src/query/models/options.go | 7 + src/query/models/options_test.go | 19 +++ src/query/models/tags.go | 51 +++++- src/query/models/tags_test.go | 34 ++++ src/query/models/types.go | 2 + src/query/test/block.go | 34 +++- 28 files changed, 949 insertions(+), 288 deletions(-) create mode 100644 src/query/block/block_test.go create mode 100644 src/query/block/empty.go create mode 100644 src/query/block/empty_test.go create mode 100644 src/query/block/meta.go create mode 100644 src/query/block/meta_test.go create mode 100644 src/query/functions/aggregation/absent_test.go delete mode 100644 src/query/functions/linear/base.go diff --git a/src/query/block/block_test.go b/src/query/block/block_test.go new file mode 100644 index 0000000000..26ee048103 --- /dev/null +++ b/src/query/block/block_test.go @@ -0,0 +1,80 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package block + +import ( + "fmt" + "testing" + + "github.com/m3db/m3/src/query/models" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// MustMakeTags creates tags given that the number of args is even. +func MustMakeTags(tag ...string) models.Tags { + if len(tag)%2 != 0 { + panic("must have even tag length") + } + + tagLength := len(tag) / 2 + t := models.NewTags(tagLength, models.NewTagOptions()) + for i := 0; i < tagLength; i++ { + t = t.AddTag(models.Tag{ + Name: []byte(tag[i*2]), + Value: []byte(tag[i*2+1]), + }) + } + + return t +} + +// MustMakeMeta creates metadata with given bounds and tags provided the number +// is even. +func MustMakeMeta(bounds models.Bounds, tags ...string) Metadata { + return Metadata{ + Tags: MustMakeTags(tags...), + Bounds: bounds, + } +} + +// MustMakeSeriesMeta creates series metadata with given bounds and tags +// provided the number is even. +func MustMakeSeriesMeta(tags ...string) SeriesMeta { + return SeriesMeta{ + Tags: MustMakeTags(tags...), + } +} + +func CompareMeta(t *testing.T, ex, ac Metadata) { + expectedTags := ex.Tags.Tags + actualTags := ac.Tags.Tags + require.Equal(t, len(expectedTags), len(actualTags)) + for i, tag := range expectedTags { + fmt.Println("x", string(tag.Name), ":", string(tag.Value)) + fmt.Println("a", string(actualTags[i].Name), ":", string(actualTags[i].Value)) + assert.Equal(t, string(tag.Name), string(actualTags[i].Name)) + assert.Equal(t, string(tag.Value), string(actualTags[i].Value)) + } + + assert.True(t, ex.Bounds.Equals(ac.Bounds)) +} diff --git a/src/query/block/column.go b/src/query/block/column.go index 6e171e1631..9b38da5c5d 100644 --- a/src/query/block/column.go +++ b/src/query/block/column.go @@ -292,8 +292,8 @@ func (m *columnBlockSeriesIter) Next() bool { } cols := m.columns - for i := 0; i < len(cols); i++ { - m.values[i] = cols[i].Values[m.idx] + for i, col := range cols { + m.values[i] = col.Values[m.idx] } return next diff --git a/src/query/block/empty.go b/src/query/block/empty.go new file mode 100644 index 0000000000..92de064156 --- /dev/null +++ b/src/query/block/empty.go @@ -0,0 +1,130 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package block + +type emptyBlock struct { + meta Metadata +} + +// NewEmptyBlock creates an empty block with the given metadata. +func NewEmptyBlock(meta Metadata) Block { + return &emptyBlock{meta: meta} +} + +func (b *emptyBlock) Close() error { return nil } + +func (b *emptyBlock) WithMetadata(meta Metadata, _ []SeriesMeta) (Block, error) { + return NewEmptyBlock(meta), nil +} + +func (b *emptyBlock) StepIter() (StepIter, error) { + return &emptyStepIter{meta: b.meta}, nil +} + +type emptyStepIter struct { + meta Metadata +} + +func (it *emptyStepIter) Close() {} +func (it *emptyStepIter) Err() error { return nil } +func (it *emptyStepIter) StepCount() int { return it.meta.Bounds.Steps() } +func (it *emptyStepIter) SeriesMeta() []SeriesMeta { return []SeriesMeta{} } +func (it *emptyStepIter) Next() bool { return false } +func (it *emptyStepIter) Meta() Metadata { return it.meta } +func (it *emptyStepIter) Current() Step { return nil } + +func (b *emptyBlock) SeriesIter() (SeriesIter, error) { + return &emptySeriesIter{ + meta: b.meta, + }, nil +} + +type emptySeriesIter struct { + meta Metadata +} + +func (it *emptySeriesIter) Close() {} +func (it *emptySeriesIter) Err() error { return nil } +func (it *emptySeriesIter) SeriesCount() int { return 0 } +func (it *emptySeriesIter) SeriesMeta() []SeriesMeta { return []SeriesMeta{} } +func (it *emptySeriesIter) Next() bool { return false } +func (it *emptySeriesIter) Current() Series { return Series{} } +func (it *emptySeriesIter) Meta() Metadata { return it.meta } + +// Unconsolidated returns the unconsolidated version for the block +func (b *emptyBlock) Unconsolidated() (UnconsolidatedBlock, error) { + return &ucEmptyBlock{ + meta: b.meta, + }, nil +} + +type ucEmptyBlock struct { + meta Metadata +} + +func (b *ucEmptyBlock) Close() error { return nil } + +func (b *ucEmptyBlock) WithMetadata( + meta Metadata, _ []SeriesMeta) (UnconsolidatedBlock, error) { + return &ucEmptyBlock{ + meta: meta, + }, nil +} + +func (b *ucEmptyBlock) Consolidate() (Block, error) { + return NewEmptyBlock(b.meta), nil +} + +func (b *ucEmptyBlock) StepIter() (UnconsolidatedStepIter, error) { + return &ucEmptyStepIter{ + meta: b.meta, + }, nil +} + +type ucEmptyStepIter struct { + meta Metadata +} + +func (it *ucEmptyStepIter) Close() {} +func (it *ucEmptyStepIter) Err() error { return nil } +func (it *ucEmptyStepIter) StepCount() int { return it.meta.Bounds.Steps() } +func (it *ucEmptyStepIter) SeriesMeta() []SeriesMeta { return []SeriesMeta{} } +func (it *ucEmptyStepIter) Next() bool { return false } +func (it *ucEmptyStepIter) Meta() Metadata { return it.meta } +func (it *ucEmptyStepIter) Current() UnconsolidatedStep { return nil } + +func (b *ucEmptyBlock) SeriesIter() (UnconsolidatedSeriesIter, error) { + return &ucEmptySeriesIter{ + meta: b.meta, + }, nil +} + +type ucEmptySeriesIter struct { + meta Metadata +} + +func (it *ucEmptySeriesIter) Close() {} +func (it *ucEmptySeriesIter) Err() error { return nil } +func (it *ucEmptySeriesIter) SeriesCount() int { return 0 } +func (it *ucEmptySeriesIter) SeriesMeta() []SeriesMeta { return []SeriesMeta{} } +func (it *ucEmptySeriesIter) Next() bool { return false } +func (it *ucEmptySeriesIter) Current() UnconsolidatedSeries { return UnconsolidatedSeries{} } +func (it *ucEmptySeriesIter) Meta() Metadata { return it.meta } diff --git a/src/query/block/empty_test.go b/src/query/block/empty_test.go new file mode 100644 index 0000000000..5857f066ab --- /dev/null +++ b/src/query/block/empty_test.go @@ -0,0 +1,103 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package block + +import ( + "testing" + "time" + + "github.com/m3db/m3/src/query/models" + + "github.com/stretchr/testify/assert" +) + +var ( + emptyArgs = []interface{}{} + start = time.Now() + steps = 15 + testBound = models.Bounds{ + Start: start, + Duration: time.Minute * time.Duration(steps), + StepSize: time.Minute, + } +) + +func mustMakeMeta(tags ...string) Metadata { + return MustMakeMeta(testBound, tags...) +} + +func TestEmptyBlock(t *testing.T) { + meta := mustMakeMeta("a", "b") + bl := NewEmptyBlock(meta) + + series, err := bl.SeriesIter() + assert.NoError(t, err) + assert.NoError(t, series.Err()) + assert.False(t, series.Next()) + assert.Equal(t, 0, series.SeriesCount()) + assert.Equal(t, []SeriesMeta{}, series.SeriesMeta()) + assert.True(t, meta.Equals(series.Meta())) + + step, err := bl.StepIter() + assert.NoError(t, err) + assert.NoError(t, step.Err()) + assert.False(t, step.Next()) + assert.Equal(t, steps, step.StepCount()) + assert.Equal(t, []SeriesMeta{}, step.SeriesMeta()) + assert.True(t, meta.Equals(step.Meta())) + + assert.NotPanics(t, func() { + series.Close() + step.Close() + }) + + assert.NoError(t, bl.Close()) +} + +func TestEmptyUnconsolidatedBlock(t *testing.T) { + meta := mustMakeMeta("a", "b") + b := NewEmptyBlock(meta) + bl, err := b.Unconsolidated() + assert.NoError(t, err) + + series, err := bl.SeriesIter() + assert.NoError(t, err) + assert.NoError(t, series.Err()) + assert.False(t, series.Next()) + assert.Equal(t, 0, series.SeriesCount()) + assert.Equal(t, []SeriesMeta{}, series.SeriesMeta()) + assert.True(t, meta.Equals(series.Meta())) + + step, err := bl.StepIter() + assert.NoError(t, err) + assert.NoError(t, step.Err()) + assert.False(t, step.Next()) + assert.Equal(t, steps, step.StepCount()) + assert.Equal(t, []SeriesMeta{}, step.SeriesMeta()) + assert.True(t, meta.Equals(step.Meta())) + + assert.NotPanics(t, func() { + series.Close() + step.Close() + }) + + assert.NoError(t, bl.Close()) +} diff --git a/src/query/block/meta.go b/src/query/block/meta.go new file mode 100644 index 0000000000..afa5dcf4dc --- /dev/null +++ b/src/query/block/meta.go @@ -0,0 +1,45 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package block + +import ( + "fmt" + + "github.com/m3db/m3/src/query/models" +) + +// Metadata is metadata for a block, describing size and common tags accross +// constituent series. +type Metadata struct { + Bounds models.Bounds + Tags models.Tags // Common tags across different series +} + +// Equals returns a boolean reporting whether the compared metadata has equal +// fields. +func (m Metadata) Equals(other Metadata) bool { + return m.Tags.Equals(other.Tags) && m.Bounds.Equals(other.Bounds) +} + +// String returns a string representation of metadata. +func (m Metadata) String() string { + return fmt.Sprintf("Bounds: %v, Tags: %v", m.Bounds, m.Tags) +} diff --git a/src/query/block/meta_test.go b/src/query/block/meta_test.go new file mode 100644 index 0000000000..f7a2bd5b64 --- /dev/null +++ b/src/query/block/meta_test.go @@ -0,0 +1,37 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package block + +import ( + "testing" + + "github.com/m3db/m3/src/query/models" + + "github.com/stretchr/testify/assert" +) + +func TestMeta(t *testing.T) { + bounds, otherBounds := models.Bounds{}, models.Bounds{} + badBounds := models.Bounds{Duration: 100} + + assert.True(t, bounds.Equals(otherBounds)) + assert.False(t, bounds.Equals(badBounds)) +} diff --git a/src/query/block/scalar.go b/src/query/block/scalar.go index 6bf6e3a3ed..0f456f5bfd 100644 --- a/src/query/block/scalar.go +++ b/src/query/block/scalar.go @@ -28,30 +28,27 @@ import ( ) // Scalar is a block containing a single value over a certain bound -// This represents constant values; it greatly simplifies downstream operations by -// allowing them to treat this as a regular block, while at the same time -// having an option to optimize by accessing the scalar value directly instead +// This represents constant values; it greatly simplifies downstream operations +// vy allowing them to treat this as a regular block, while at the same time +// having an option to optimize by accessing the scalar value directly instead. type Scalar struct { s ScalarFunc meta Metadata } -// NewScalar creates a scalar block containing val over the bounds +// NewScalar creates a scalar block whose value is given by the function over +// the metadata bounds. func NewScalar( s ScalarFunc, - bounds models.Bounds, - tagOptions models.TagOptions, + meta Metadata, ) Block { return &Scalar{ - s: s, - meta: Metadata{ - Bounds: bounds, - Tags: models.NewTags(0, tagOptions), - }, + s: s, + meta: meta, } } -// Unconsolidated returns the unconsolidated version for the block +// Unconsolidated returns the unconsolidated version for the block. func (b *Scalar) Unconsolidated() (UnconsolidatedBlock, error) { return nil, fmt.Errorf("unconsolidated view not implemented for scalar block, meta: %s", b.meta) } diff --git a/src/query/block/scalar_test.go b/src/query/block/scalar_test.go index a81296cd4c..c6ddb4a745 100644 --- a/src/query/block/scalar_test.go +++ b/src/query/block/scalar_test.go @@ -31,7 +31,6 @@ import ( ) var ( - start = time.Time{} val = 13.37 bounds = models.Bounds{ Start: start, @@ -41,10 +40,18 @@ var ( ) func TestScalarBlock(t *testing.T) { + tagOpts := models.NewTagOptions().SetBucketName([]byte("custom_bucket")) + tags := models.NewTags(1, tagOpts).AddTag(models.Tag{ + Name: []byte("a"), + Value: []byte("b"), + }) + block := NewScalar( func(_ time.Time) float64 { return val }, - bounds, - models.NewTagOptions(), + Metadata{ + Bounds: bounds, + Tags: tags, + }, ) require.IsType(t, block, &Scalar{}) @@ -52,7 +59,7 @@ func TestScalarBlock(t *testing.T) { require.NoError(t, err) require.NotNil(t, stepIter) - verifyMetas(t, stepIter.Meta(), stepIter.SeriesMeta()) + verifyMetas(t, stepIter.Meta(), stepIter.SeriesMeta(), tagOpts) assert.Equal(t, 6, stepIter.StepCount()) valCounts := 0 @@ -77,7 +84,7 @@ func TestScalarBlock(t *testing.T) { require.NoError(t, err) require.NotNil(t, seriesIter) - verifyMetas(t, seriesIter.Meta(), seriesIter.SeriesMeta()) + verifyMetas(t, seriesIter.Meta(), seriesIter.SeriesMeta(), tagOpts) require.Equal(t, 1, seriesIter.SeriesCount()) require.True(t, seriesIter.Next()) @@ -100,10 +107,14 @@ func TestScalarBlock(t *testing.T) { require.NoError(t, block.Close()) } -func verifyMetas(t *testing.T, meta Metadata, seriesMeta []SeriesMeta) { +func verifyMetas( + t *testing.T, + meta Metadata, + seriesMeta []SeriesMeta, + opts models.TagOptions, +) { // Verify meta - assert.Equal(t, bounds, meta.Bounds) - assert.Equal(t, 0, meta.Tags.Len()) + assert.True(t, bounds.Equals(meta.Bounds)) // Verify seriesMeta assert.Len(t, seriesMeta, 1) diff --git a/src/query/block/types.go b/src/query/block/types.go index f536648d58..2fea02b39a 100644 --- a/src/query/block/types.go +++ b/src/query/block/types.go @@ -21,7 +21,6 @@ package block import ( - "fmt" "io" "math" "time" @@ -152,17 +151,6 @@ type UnconsolidatedStep interface { Values() []ts.Datapoints } -// Metadata is metadata for a block. -type Metadata struct { - Bounds models.Bounds - Tags models.Tags // Common tags across different series -} - -// String returns a string representation of metadata. -func (m Metadata) String() string { - return fmt.Sprintf("Bounds: %v, Tags: %v", m.Bounds, m.Tags) -} - // Builder builds a new block. type Builder interface { AppendValue(idx int, value float64) error diff --git a/src/query/executor/transform/exec_test.go b/src/query/executor/transform/exec_test.go index 861a0fb2b2..5c889f84cd 100644 --- a/src/query/executor/transform/exec_test.go +++ b/src/query/executor/transform/exec_test.go @@ -24,6 +24,7 @@ import ( "context" "errors" "testing" + "time" "github.com/m3db/m3/src/query/block" "github.com/m3db/m3/src/query/functions/utils" @@ -58,11 +59,16 @@ func TestProcessSimpleBlock(t *testing.T) { child := NewMockOpNode(ctrl) controller.AddTransform(child) + step := time.Second + bounds := models.Bounds{ + StepSize: step, + Duration: step, + } + return &testContext{ - MockCtrl: ctrl, - Controller: controller, - SourceBlock: test.NewBlockFromValues( - models.Bounds{}, [][]float64{{1.0}}), + MockCtrl: ctrl, + Controller: controller, + SourceBlock: test.NewBlockFromValues(bounds, [][]float64{{1.0}}), ResultBlock: block.NewMockBlock(ctrl), Node: NewMocksimpleOpNode(ctrl), ChildNode: child, diff --git a/src/query/functions/aggregation/absent.go b/src/query/functions/aggregation/absent.go index ed44ec99bc..85a8fe8d07 100644 --- a/src/query/functions/aggregation/absent.go +++ b/src/query/functions/aggregation/absent.go @@ -53,7 +53,7 @@ func (o absentOp) OpType() string { // String representation. func (o absentOp) String() string { - return fmt.Sprintf("type: absent") + return "type: absent" } // Node creates an execution node. @@ -82,7 +82,6 @@ func (n *absentNode) Params() parser.Params { return n.op } -// Process the block func (n *absentNode) Process(queryCtx *models.QueryContext, ID parser.NodeID, b block.Block) error { return transform.ProcessSimpleBlock(n, n.controller, queryCtx, ID, b) @@ -95,7 +94,6 @@ func (n *absentNode) ProcessBlock(queryCtx *models.QueryContext, return nil, err } - // Absent should var ( meta = stepIter.Meta() seriesMetas = stepIter.SeriesMeta() @@ -106,45 +104,69 @@ func (n *absentNode) ProcessBlock(queryCtx *models.QueryContext, if len(seriesMetas) == 0 { return block.NewScalar( func(_ time.Time) float64 { return 1 }, - meta.Bounds, - tagOpts, + meta, ), nil } // NB: pull any common tags out into the created series. dupeTags, _ := utils.DedupeMetadata(seriesMetas, tagOpts) - mergedCommonTags := meta.Tags.Add(dupeTags) - meta.Tags = models.NewTags(0, tagOpts) + meta.Tags = meta.Tags.Add(dupeTags).Normalize() emptySeriesMeta := []block.SeriesMeta{ block.SeriesMeta{ - Tags: mergedCommonTags, + Tags: models.NewTags(0, tagOpts), Name: []byte{}, }, } - builder, err := n.controller.BlockBuilder(queryCtx, meta, emptySeriesMeta) - if err != nil { - return nil, err - } + setupBuilderWithValuesToIndex := func(idx int) (block.Builder, error) { + builder, err := n.controller.BlockBuilder(queryCtx, meta, emptySeriesMeta) + if err != nil { + return nil, err + } - if err = builder.AddCols(1); err != nil { - return nil, err - } + if err = builder.AddCols(stepIter.StepCount()); err != nil { + return nil, err + } - for index := 0; stepIter.Next(); index++ { - step := stepIter.Current() - values := step.Values() + for i := 0; i < idx; i++ { + if err := builder.AppendValue(i, math.NaN()); err != nil { + return nil, err + } + } - var val float64 = 1 + if err := builder.AppendValue(idx, 1); err != nil { + return nil, err + } + + return builder, err + } + + var builder block.Builder + for idx := 0; stepIter.Next(); idx++ { + var ( + step = stepIter.Current() + values = step.Values() + val float64 = 1 + ) + fmt.Println(values) for _, v := range values { if !math.IsNaN(v) { - val = 0 + val = math.NaN() break } } - if err := builder.AppendValue(index, val); err != nil { - return nil, err + if builder == nil { + if !math.IsNaN(val) { + builder, err = setupBuilderWithValuesToIndex(idx) + if err != nil { + return nil, err + } + } + } else { + if err := builder.AppendValue(idx, val); err != nil { + return nil, err + } } } @@ -152,5 +174,9 @@ func (n *absentNode) ProcessBlock(queryCtx *models.QueryContext, return nil, err } + if builder == nil { + return block.NewEmptyBlock(meta), nil + } + return builder.Build(), nil } diff --git a/src/query/functions/aggregation/absent_test.go b/src/query/functions/aggregation/absent_test.go new file mode 100644 index 0000000000..af73e21296 --- /dev/null +++ b/src/query/functions/aggregation/absent_test.go @@ -0,0 +1,160 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package aggregation + +import ( + "math" + "testing" + "time" + + "github.com/m3db/m3/src/query/block" + "github.com/m3db/m3/src/query/executor/transform" + "github.com/m3db/m3/src/query/models" + "github.com/m3db/m3/src/query/parser" + "github.com/m3db/m3/src/query/test" + "github.com/m3db/m3/src/query/test/executor" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func toArgs(f float64) []interface{} { return []interface{}{f} } + +var ( + start = time.Now() + testBound = models.Bounds{ + Start: start, + Duration: time.Hour, + StepSize: time.Minute * 15, + } +) + +func mustMakeMeta(tags ...string) block.Metadata { + return block.Metadata{ + Bounds: testBound, + Tags: test.MustMakeTags(tags...), + } +} + +func mustMakeSeriesMeta(tags ...string) block.SeriesMeta { + return block.SeriesMeta{ + Tags: test.MustMakeTags(tags...), + } +} + +var absentTests = []struct { + name string + meta block.Metadata + seriesMetas []block.SeriesMeta + vals [][]float64 + expectedMeta block.Metadata + expectedVals []float64 +}{ + { + "no series", + mustMakeMeta(), + []block.SeriesMeta{}, + [][]float64{}, + mustMakeMeta(), + []float64{1, 1, 1, 1}, + }, + { + "no series with tags", + mustMakeMeta("A", "B", "C", "D"), + []block.SeriesMeta{}, + [][]float64{}, + mustMakeMeta("A", "B", "C", "D"), + []float64{1, 1, 1, 1}, + }, + { + "series with tags and values", + mustMakeMeta("A", "B", "C", "D"), + []block.SeriesMeta{mustMakeSeriesMeta("B", "B")}, + [][]float64{{1, 1, 1, 1}}, + mustMakeMeta("A", "B", "B", "B", "C", "D"), + nil, + }, + { + "series with tags and some missing", + mustMakeMeta("A", "B", "C", "D"), + []block.SeriesMeta{mustMakeSeriesMeta("bar", "baz")}, + [][]float64{{1, 1, 1, math.NaN()}}, + mustMakeMeta("A", "B", "bar", "baz", "C", "D"), + []float64{nan, nan, nan, 1}, + }, + { + "series with mismatched tags", + mustMakeMeta("A", "B", "C", "D"), + []block.SeriesMeta{ + mustMakeSeriesMeta("B", "B"), + mustMakeSeriesMeta("F", "F"), + }, + [][]float64{ + {1, 1, 1, math.NaN()}, + {math.NaN(), 1, 1, math.NaN()}, + }, + mustMakeMeta("A", "B", "C", "D"), + []float64{nan, nan, nan, 1}, + }, + { + "series with no missing values", + mustMakeMeta("A", "B", "C", "D"), + []block.SeriesMeta{ + mustMakeSeriesMeta("F", "F"), + mustMakeSeriesMeta("F", "F"), + }, + [][]float64{ + {1, math.NaN(), math.NaN(), 2}, + {math.NaN(), 1, 1, math.NaN()}, + }, + mustMakeMeta("A", "B", "C", "D", "F", "F"), + nil, + }, +} + +func TestAbsent(t *testing.T) { + for _, tt := range absentTests { + t.Run(tt.name, func(t *testing.T) { + block := test.NewBlockFromValuesWithMetaAndSeriesMeta( + tt.meta, + tt.seriesMetas, + tt.vals, + ) + + c, sink := executor.NewControllerWithSink(parser.NodeID(1)) + absentOp := NewAbsentOp() + op, ok := absentOp.(transform.Params) + require.True(t, ok) + + node := op.Node(c, transform.Options{}) + err := node.Process(models.NoopQueryContext(), parser.NodeID(0), block) + require.NoError(t, err) + + if tt.expectedVals == nil { + require.Equal(t, 0, len(sink.Values)) + } else { + require.Equal(t, 1, len(sink.Values)) + test.EqualsWithNans(t, tt.expectedVals, sink.Values[0]) + assert.True(t, tt.expectedMeta.Equals(sink.Meta)) + } + }) + } +} diff --git a/src/query/functions/binary/binary.go b/src/query/functions/binary/binary.go index e5bd3e7a55..d13d584bfc 100644 --- a/src/query/functions/binary/binary.go +++ b/src/query/functions/binary/binary.go @@ -85,8 +85,7 @@ func processBinary( func(t time.Time) float64 { return fn(lVal, scalarR.Value(t)) }, - lIter.Meta().Bounds, - lIter.Meta().Tags.Opts, + lIter.Meta(), ), nil } diff --git a/src/query/functions/binary/binary_test.go b/src/query/functions/binary/binary_test.go index 352e49a6dc..609942a5ef 100644 --- a/src/query/functions/binary/binary_test.go +++ b/src/query/functions/binary/binary_test.go @@ -121,8 +121,10 @@ func TestScalars(t *testing.T) { parser.NodeID(0), block.NewScalar( func(_ time.Time) float64 { return tt.lVal }, - bounds, - models.NewTagOptions(), + block.Metadata{ + Bounds: bounds, + Tags: models.EmptyTags(), + }, ), ) @@ -132,8 +134,10 @@ func TestScalars(t *testing.T) { parser.NodeID(1), block.NewScalar( func(_ time.Time) float64 { return tt.rVal }, - bounds, - models.NewTagOptions(), + block.Metadata{ + Bounds: bounds, + Tags: models.EmptyTags(), + }, ), ) @@ -180,8 +184,10 @@ func TestScalarsReturnBoolFalse(t *testing.T) { parser.NodeID(0), block.NewScalar( func(_ time.Time) float64 { return tt.lVal }, - bounds, - models.NewTagOptions(), + block.Metadata{ + Bounds: bounds, + Tags: models.EmptyTags(), + }, ), ) @@ -191,8 +197,10 @@ func TestScalarsReturnBoolFalse(t *testing.T) { parser.NodeID(1), block.NewScalar( func(_ time.Time) float64 { return tt.rVal }, - bounds, - models.NewTagOptions(), + block.Metadata{ + Bounds: bounds, + Tags: models.EmptyTags(), + }, ), ) @@ -586,8 +594,10 @@ func TestSingleSeriesReturnBool(t *testing.T) { parser.NodeID(1), block.NewScalar( func(_ time.Time) float64 { return tt.scalarVal }, - bounds, - models.NewTagOptions(), + block.Metadata{ + Bounds: bounds, + Tags: models.EmptyTags(), + }, ), ) @@ -598,8 +608,10 @@ func TestSingleSeriesReturnBool(t *testing.T) { parser.NodeID(0), block.NewScalar( func(_ time.Time) float64 { return tt.scalarVal }, - bounds, - models.NewTagOptions(), + block.Metadata{ + Bounds: bounds, + Tags: models.EmptyTags(), + }, ), ) @@ -658,8 +670,10 @@ func TestSingleSeriesReturnValues(t *testing.T) { parser.NodeID(1), block.NewScalar( func(_ time.Time) float64 { return tt.scalarVal }, - bounds, - models.NewTagOptions(), + block.Metadata{ + Bounds: bounds, + Tags: models.EmptyTags(), + }, ), ) @@ -670,8 +684,10 @@ func TestSingleSeriesReturnValues(t *testing.T) { parser.NodeID(0), block.NewScalar( func(_ time.Time) float64 { return tt.scalarVal }, - bounds, - models.NewTagOptions(), + block.Metadata{ + Bounds: bounds, + Tags: models.EmptyTags(), + }, ), ) diff --git a/src/query/functions/linear/base.go b/src/query/functions/linear/base.go deleted file mode 100644 index c736504335..0000000000 --- a/src/query/functions/linear/base.go +++ /dev/null @@ -1,123 +0,0 @@ -// Copyright (c) 2018 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -package linear - -import ( - "fmt" - - "github.com/m3db/m3/src/query/block" - "github.com/m3db/m3/src/query/executor/transform" - "github.com/m3db/m3/src/query/models" - "github.com/m3db/m3/src/query/parser" -) - -var emptyOp = BaseOp{} - -// BaseOp stores required properties for logical operations -type BaseOp struct { - operatorType string - processorFn makeProcessor -} - -// OpType for the operator -func (o BaseOp) OpType() string { - return o.operatorType -} - -// String representation -func (o BaseOp) String() string { - return fmt.Sprintf("type: %s", o.OpType()) -} - -// Node creates an execution node -func (o BaseOp) Node(controller *transform.Controller, _ transform.Options) transform.OpNode { - return &baseNode{ - controller: controller, - op: o, - processor: o.processorFn(o, controller), - } -} - -type baseNode struct { - op BaseOp - controller *transform.Controller - processor Processor -} - -func (c *baseNode) Params() parser.Params { - return c.op -} - -// Process the block -func (c *baseNode) Process(queryCtx *models.QueryContext, ID parser.NodeID, b block.Block) error { - return transform.ProcessSimpleBlock(c, c.controller, queryCtx, ID, b) -} - -// ProcessBlock applies the linear function time Step-wise to each value in the block. -func (c *baseNode) ProcessBlock(queryCtx *models.QueryContext, ID parser.NodeID, b block.Block) (block.Block, error) { - stepIter, err := b.StepIter() - if err != nil { - return nil, err - } - - builder, err := c.controller.BlockBuilder(queryCtx, stepIter.Meta(), stepIter.SeriesMeta()) - if err != nil { - return nil, err - } - - if err := builder.AddCols(stepIter.StepCount()); err != nil { - return nil, err - } - - for index := 0; stepIter.Next(); index++ { - step := stepIter.Current() - values := c.processor.Process(step.Values()) - for _, value := range values { - if err := builder.AppendValue(index, value); err != nil { - return nil, err - } - } - } - - if err = stepIter.Err(); err != nil { - return nil, err - } - - return builder.Build(), nil -} - -// Meta returns the metadata for the block -func (c *baseNode) Meta(meta block.Metadata) block.Metadata { - return meta -} - -// SeriesMeta returns the metadata for each series in the block -func (c *baseNode) SeriesMeta(metas []block.SeriesMeta) []block.SeriesMeta { - return metas -} - -// makeProcessor is a way to create a transform -type makeProcessor func(op BaseOp, controller *transform.Controller) Processor - -// Processor is implemented by the underlying transforms -type Processor interface { - Process(values []float64) []float64 -} diff --git a/src/query/functions/linear/clamp.go b/src/query/functions/linear/clamp.go index 394fcf7875..b300c036ae 100644 --- a/src/query/functions/linear/clamp.go +++ b/src/query/functions/linear/clamp.go @@ -24,14 +24,18 @@ import ( "fmt" "math" - "github.com/m3db/m3/src/query/executor/transform" + "github.com/m3db/m3/src/query/block" + "github.com/m3db/m3/src/query/functions/lazy" + "github.com/m3db/m3/src/query/parser" ) const ( - // ClampMinType ensures all values except NaNs are greater than or equal to the provided argument + // ClampMinType ensures all values except NaNs are greater + // than or equal to the provided argument. ClampMinType = "clamp_min" - // ClampMaxType ensures all values except NaNs are lesser than or equal to provided argument + // ClampMaxType ensures all values except NaNs are lesser + // than or equal to provided argument. ClampMaxType = "clamp_max" ) @@ -40,55 +44,40 @@ type clampOp struct { scalar float64 } -// NewClampOp creates a new clamp op based on the type and arguments -func NewClampOp(args []interface{}, optype string) (BaseOp, error) { +func parseClampArgs(args []interface{}) (float64, error) { if len(args) != 1 { - return emptyOp, fmt.Errorf("invalid number of args for clamp: %d", len(args)) - } - - if optype != ClampMinType && optype != ClampMaxType { - return emptyOp, fmt.Errorf("unknown clamp type: %s", optype) + return 0, fmt.Errorf("invalid number of args for clamp: %d", len(args)) } scalar, ok := args[0].(float64) if !ok { - return emptyOp, fmt.Errorf("unable to cast to scalar argument: %v", args[0]) - } - - spec := clampOp{ - opType: optype, - scalar: scalar, + return 0, fmt.Errorf("unable to cast to scalar argument: %v", args[0]) } - return BaseOp{ - operatorType: optype, - processorFn: makeClampProcessor(spec), - }, nil + return scalar, nil } -func makeClampProcessor(spec clampOp) makeProcessor { - clampOp := spec - return func(op BaseOp, controller *transform.Controller) Processor { - fn := math.Min - if op.operatorType == ClampMinType { - fn = math.Max - } - - return &clampNode{op: clampOp, controller: controller, clampFn: fn} +func clampFn(max bool, roundTo float64) block.ValueTransform { + if max { + return func(v float64) float64 { return math.Min(v, roundTo) } } -} -type clampNode struct { - op clampOp - clampFn func(x, y float64) float64 - controller *transform.Controller + return func(v float64) float64 { return math.Max(v, roundTo) } } -func (c *clampNode) Process(values []float64) []float64 { - scalar := c.op.scalar - for i := range values { - values[i] = c.clampFn(values[i], scalar) +// NewClampOp creates a new clamp op based on the type and arguments +func NewClampOp(args []interface{}, opType string) (parser.Params, error) { + isMax := opType == ClampMaxType + if opType != ClampMinType && !isMax { + return nil, fmt.Errorf("unknown clamp type: %s", opType) + } + + clampTo, err := parseClampArgs(args) + if err != nil { + return nil, err } - return values + fn := clampFn(isMax, clampTo) + lazyOpts := block.NewLazyOptions().SetValueTransform(fn) + return lazy.NewLazyOp(opType, lazyOpts) } diff --git a/src/query/functions/linear/clamp_test.go b/src/query/functions/linear/clamp_test.go index 088b1cea63..bf11303de4 100644 --- a/src/query/functions/linear/clamp_test.go +++ b/src/query/functions/linear/clamp_test.go @@ -23,6 +23,7 @@ package linear import ( "math" "testing" + "time" "github.com/m3db/m3/src/query/executor/transform" "github.com/m3db/m3/src/query/models" @@ -54,8 +55,12 @@ func TestClampMin(t *testing.T) { block := test.NewBlockFromValues(bounds, values) c, sink := executor.NewControllerWithSink(parser.NodeID(1)) - op, err := NewClampOp([]interface{}{3.0}, ClampMinType) + clampOp, err := NewClampOp([]interface{}{3.0}, ClampMinType) require.NoError(t, err) + + op, ok := clampOp.(transform.Params) + require.True(t, ok) + node := op.Node(c, transform.Options{}) err = node.Process(models.NoopQueryContext(), parser.NodeID(0), block) require.NoError(t, err) @@ -70,8 +75,12 @@ func TestClampMax(t *testing.T) { block := test.NewBlockFromValues(bounds, values) c, sink := executor.NewControllerWithSink(parser.NodeID(1)) - op, err := NewClampOp([]interface{}{3.0}, ClampMaxType) + clampOp, err := NewClampOp([]interface{}{3.0}, ClampMaxType) require.NoError(t, err) + + op, ok := clampOp.(transform.Params) + require.True(t, ok) + node := op.Node(c, transform.Options{}) err = node.Process(models.NoopQueryContext(), parser.NodeID(0), block) require.NoError(t, err) @@ -79,3 +88,46 @@ func TestClampMax(t *testing.T) { assert.Len(t, sink.Values, 2) test.EqualsWithNans(t, expected, sink.Values) } + +func TestClampFailsParse(t *testing.T) { + _, err := NewClampOp([]interface{}{}, "bad") + assert.Error(t, err) +} + +func runClamp(t *testing.T, args []interface{}, + opType string, vals []float64) []float64 { + bounds := models.Bounds{ + StepSize: step, + Duration: step * time.Duration(len(vals)), + } + + v := [][]float64{vals} + block := test.NewBlockFromValues(bounds, v) + c, sink := executor.NewControllerWithSink(parser.NodeID(1)) + roundOp, err := NewClampOp(args, opType) + require.NoError(t, err) + + op, ok := roundOp.(transform.Params) + require.True(t, ok) + + node := op.Node(c, transform.Options{}) + err = node.Process(models.NoopQueryContext(), parser.NodeID(0), block) + require.NoError(t, err) + require.Len(t, sink.Values, 1) + + return sink.Values[0] +} + +func TestClampWithArgs(t *testing.T) { + var ( + v = []float64{math.NaN(), 0, 1, 2, 3, math.Inf(1), math.Inf(-1)} + exMax = []float64{math.NaN(), 0, 1, 2, 2, 2, math.Inf(-1)} + exMin = []float64{math.NaN(), 2, 2, 2, 3, math.Inf(1), 2} + ) + + max := runClamp(t, toArgs(2), ClampMaxType, v) + test.EqualsWithNans(t, exMax, max) + + min := runClamp(t, toArgs(2), ClampMinType, v) + test.EqualsWithNans(t, exMin, min) +} diff --git a/src/query/functions/linear/histogram_quantile.go b/src/query/functions/linear/histogram_quantile.go index 8f89138675..b9702708ac 100644 --- a/src/query/functions/linear/histogram_quantile.go +++ b/src/query/functions/linear/histogram_quantile.go @@ -50,17 +50,17 @@ func NewHistogramQuantileOp( opType string, ) (parser.Params, error) { if len(args) != 1 { - return emptyOp, fmt.Errorf( + return nil, fmt.Errorf( "invalid number of args for histogram_quantile: %d", len(args)) } if opType != HistogramQuantileType { - return emptyOp, fmt.Errorf("operator not supported: %s", opType) + return nil, fmt.Errorf("operator not supported: %s", opType) } q, ok := args[0].(float64) if !ok { - return emptyOp, fmt.Errorf("unable to cast to scalar argument: %v", args[0]) + return nil, fmt.Errorf("unable to cast to scalar argument: %v", args[0]) } return newHistogramQuantileOp(q, opType), nil diff --git a/src/query/functions/linear/math.go b/src/query/functions/linear/math.go index ab56bbd47a..d480e070f5 100644 --- a/src/query/functions/linear/math.go +++ b/src/query/functions/linear/math.go @@ -82,5 +82,5 @@ func NewMathOp(opType string) (parser.Params, error) { return lazy.NewLazyOp(opType, lazyOpts) } - return emptyOp, fmt.Errorf("unknown math type: %s", opType) + return nil, fmt.Errorf("unknown math type: %s", opType) } diff --git a/src/query/functions/linear/round_test.go b/src/query/functions/linear/round_test.go index 937d40e5dd..975ca8ae7b 100644 --- a/src/query/functions/linear/round_test.go +++ b/src/query/functions/linear/round_test.go @@ -21,7 +21,9 @@ package linear import ( + "math" "testing" + "time" "github.com/m3db/m3/src/query/executor/transform" "github.com/m3db/m3/src/query/models" @@ -32,35 +34,42 @@ import ( "github.com/stretchr/testify/require" ) -var emptyArgs = []interface{}{} +var ( + emptyArgs = []interface{}{} + nan = math.NaN() + step = time.Second + tests = []struct { + name string + v []float64 + args []interface{} + expected []float64 + }{ + {"default", []float64{1.2, 4.5, 6, nan}, + emptyArgs, []float64{1, 5, 6, nan}}, -func toArgs(f float64) []interface{} { return []interface{}{f} } - -var tests = []struct { - name string - v []float64 - args []interface{} - expected []float64 -}{ - {"default", []float64{1.2, 4.5, 6, nan}, - emptyArgs, []float64{1, 5, 6, nan}}, + {"1.2", []float64{1.2, 4.5, 6, nan}, + toArgs(1.2), []float64{1.2, 4.8, 6, nan}}, - {"1.2", []float64{1.2, 4.5, 6, nan}, - toArgs(1.2), []float64{1.2, 4.8, 6, nan}}, + {"-3", []float64{1.2, 4.5, 6, nan}, + toArgs(-3), []float64{0, 3, 6, nan}}, - {"-3", []float64{1.2, 4.5, 6, nan}, - toArgs(-3), []float64{0, 3, 6, nan}}, + {"0", []float64{1.2, 4.5, 6, nan}, + toArgs(0), []float64{nan, nan, nan, nan}}, + } +) - {"0", []float64{1.2, 4.5, 6, nan}, - toArgs(0), []float64{nan, nan, nan, nan}}, -} +func toArgs(f float64) []interface{} { return []interface{}{f} } func TestRoundWithArgs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + bounds := models.Bounds{ + StepSize: step, + Duration: step * time.Duration(len(tt.v)), + } + v := [][]float64{tt.v} - values, bounds := test.GenerateValuesAndBounds(v, nil) - block := test.NewBlockFromValues(bounds, values) + block := test.NewBlockFromValues(bounds, v) c, sink := executor.NewControllerWithSink(parser.NodeID(1)) roundOp, err := NewRoundOp(tt.args) require.NoError(t, err) diff --git a/src/query/functions/scalar/base.go b/src/query/functions/scalar/base.go index 71be6281a7..2faf4402fb 100644 --- a/src/query/functions/scalar/base.go +++ b/src/query/functions/scalar/base.go @@ -71,7 +71,7 @@ func (o baseOp) Node( } } -// NewScalarOp creates a new scalar op +// NewScalarOp creates a new scalar op. func NewScalarOp( fn block.ScalarFunc, opType string, @@ -97,9 +97,12 @@ type baseNode struct { // Execute runs the scalar node operation func (n *baseNode) Execute(queryCtx *models.QueryContext) error { - bounds := n.opts.TimeSpec().Bounds() + meta := block.Metadata{ + Bounds: n.opts.TimeSpec().Bounds(), + Tags: models.NewTags(0, n.op.tagOptions), + } - block := block.NewScalar(n.op.fn, bounds, n.op.tagOptions) + block := block.NewScalar(n.op.fn, meta) if n.opts.Debug() { // Ignore any errors iter, _ := block.StepIter() diff --git a/src/query/models/bounds.go b/src/query/models/bounds.go index a8e3902527..8d08eb08a7 100644 --- a/src/query/models/bounds.go +++ b/src/query/models/bounds.go @@ -25,14 +25,15 @@ import ( "time" ) -// Bounds are the time bounds, start time is inclusive but end is exclusive +// Bounds are the time bounds, start time is inclusive but end is exclusive. type Bounds struct { Start time.Time Duration time.Duration StepSize time.Duration } -// TimeForIndex returns the start time for a given index assuming a uniform step size +// TimeForIndex returns the start time for a given index assuming +// a uniform step size. func (b Bounds) TimeForIndex(idx int) (time.Time, error) { duration := time.Duration(idx) * b.StepSize if b.Steps() == 0 || duration >= b.Duration { @@ -42,12 +43,12 @@ func (b Bounds) TimeForIndex(idx int) (time.Time, error) { return b.Start.Add(duration), nil } -// End calculates the end time for the block and is exclusive +// End calculates the end time for the block and is exclusive. func (b Bounds) End() time.Time { return b.Start.Add(b.Duration) } -// Steps calculates the number of steps for the bounds +// Steps calculates the number of steps for the bounds. func (b Bounds) Steps() int { if b.StepSize <= 0 { return 0 @@ -62,12 +63,12 @@ func (b Bounds) Contains(t time.Time) bool { return diff >= 0 && diff < b.Duration } -// Next returns the nth next bound from the current bound +// Next returns the nth next bound from the current bound. func (b Bounds) Next(n int) Bounds { return b.nth(n, true) } -// Previous returns the nth previous bound from the current bound +// Previous returns the nth previous bound from the current bound. func (b Bounds) Previous(n int) Bounds { return b.nth(n, false) } @@ -87,17 +88,18 @@ func (b Bounds) nth(n int, forward bool) Bounds { } } -// Blocks returns the number of blocks until time t +// Blocks returns the number of blocks until time t. func (b Bounds) Blocks(t time.Time) int { return int(b.Start.Sub(t) / b.Duration) } -// String representation of the bounds +// String representation of the bounds. func (b Bounds) String() string { - return fmt.Sprintf("start: %v, duration: %v, stepSize: %v, steps: %d", b.Start, b.Duration, b.StepSize, b.Steps()) + return fmt.Sprintf("start: %v, duration: %v, stepSize: %v, steps: %d", + b.Start, b.Duration, b.StepSize, b.Steps()) } -// Nearest returns the nearest bound before the given time +// Nearest returns the nearest bound before the given time. func (b Bounds) Nearest(t time.Time) Bounds { startTime := b.Start duration := b.Duration @@ -128,7 +130,7 @@ func (b Bounds) Nearest(t time.Time) Bounds { } } -// Equals is true if two bounds are equal, including stepsize +// Equals is true if two bounds are equal, including step size. func (b Bounds) Equals(other Bounds) bool { if b.StepSize != other.StepSize { return false diff --git a/src/query/models/options.go b/src/query/models/options.go index f0617e0eb2..8fc8702979 100644 --- a/src/query/models/options.go +++ b/src/query/models/options.go @@ -21,6 +21,7 @@ package models import ( + "bytes" "errors" ) @@ -90,3 +91,9 @@ func (o *tagOptions) SetIDSchemeType(scheme IDSchemeType) TagOptions { func (o *tagOptions) IDSchemeType() IDSchemeType { return o.idScheme } + +func (o *tagOptions) Equals(other TagOptions) bool { + return o.idScheme == other.IDSchemeType() && + bytes.Equal(o.metricName, other.MetricName()) && + bytes.Equal(o.bucketName, other.BucketName()) +} diff --git a/src/query/models/options_test.go b/src/query/models/options_test.go index de27b54fda..0014ccd93a 100644 --- a/src/query/models/options_test.go +++ b/src/query/models/options_test.go @@ -74,3 +74,22 @@ func TestBadSchemeTagOptions(t *testing.T) { SetIDSchemeType(IDSchemeType(6)) assert.EqualError(t, opts.Validate(), msg) } + +func TestOptionsEquals(t *testing.T) { + opts, other := NewTagOptions(), NewTagOptions() + assert.True(t, opts.Equals(other)) + + bad := []byte("aaa") + n := opts.BucketName() + opts = opts.SetBucketName(bad) + assert.False(t, opts.Equals(other)) + + opts = opts.SetBucketName(n) + n = opts.MetricName() + opts = opts.SetMetricName(bad) + assert.False(t, opts.Equals(other)) + + opts = opts.SetMetricName(n) + opts = opts.SetIDSchemeType(IDSchemeType(10)) + assert.False(t, opts.Equals(other)) +} diff --git a/src/query/models/tags.go b/src/query/models/tags.go index 475be93d87..fd9db38ff5 100644 --- a/src/query/models/tags.go +++ b/src/query/models/tags.go @@ -431,24 +431,69 @@ func (t Tags) Normalize() Tags { return t } +// HashedID returns the hashed ID for the tags. +func (t Tags) Reset() Tags { + t.Tags = t.Tags[:0] + return t +} + // HashedID returns the hashed ID for the tags. func (t Tags) HashedID() uint64 { return xxhash.Sum64(t.ID()) } +// Equals returns a boolean reporting whether the compared tags have the same +// values. +// +// NB: does not check that compared tags have the same underlying bytes. +func (t Tags) Equals(other Tags) bool { + if t.Len() != other.Len() { + return false + } + + if !t.Opts.Equals(other.Opts) { + return false + } + + for i, t := range t.Tags { + if !t.Equals(other.Tags[i]) { + return false + } + } + + return true +} + +var tagSeperator = []byte(", ") + +// String returns the string representation of the tags. func (t Tags) String() string { - tags := make([]string, len(t.Tags)) + var sb strings.Builder + // tags := make([]string, len(t.Tags)) for i, tt := range t.Tags { - tags[i] = tt.String() + if i != 0 { + sb.Write(tagSeperator) + } + + sb.WriteString(tt.String()) } - return strings.Join(tags, ", ") + return sb.String() + // return strings.Join(tags, ", ") } +// String returns the string representation of the tag. func (t Tag) String() string { return fmt.Sprintf("%s: %s", t.Name, t.Value) } +// Equals returns a boolean indicating whether the provided tags are equal. +// +// NB: does not check that compared tags have the same underlying bytes. +func (t Tag) Equals(other Tag) bool { + return bytes.Equal(t.Name, other.Name) && bytes.Equal(t.Value, other.Value) +} + // Clone returns a copy of the tag. func (t Tag) Clone() Tag { // Todo: Pool these diff --git a/src/query/models/tags_test.go b/src/query/models/tags_test.go index 4141bc4868..d83443a68e 100644 --- a/src/query/models/tags_test.go +++ b/src/query/models/tags_test.go @@ -313,6 +313,9 @@ func TestCloneTags(t *testing.T) { assert.Equal(t, cloned.Opts, tags.Opts) assert.Equal(t, cloned.Tags, tags.Tags) + assert.True(t, cloned.Equals(tags)) + assert.True(t, tags.Equals(cloned)) + aHeader := (*reflect.SliceHeader)(unsafe.Pointer(&cloned.Tags)) bHeader := (*reflect.SliceHeader)(unsafe.Pointer(&tags.Tags)) assert.False(t, aHeader.Data == bHeader.Data) @@ -326,6 +329,37 @@ func TestCloneTags(t *testing.T) { assert.False(t, xtest.ByteSlicesBackedBySameData(tv, cv)) } +func TestTagsEquals(t *testing.T) { + tags, other := createTags(true), createTags(true) + assert.True(t, tags.Equals(other)) + + bad := []byte("a") + n := tags.Opts.BucketName() + tags.Opts = tags.Opts.SetBucketName(bad) + assert.False(t, tags.Equals(other)) + + tags.Opts = tags.Opts.SetBucketName(n) + assert.True(t, tags.Equals(other)) + + n = tags.Tags[0].Name + tags.Tags[0].Name = bad + assert.False(t, tags.Equals(other)) + + tags.Tags[0].Name = n + assert.True(t, tags.Equals(other)) + + tags = tags.AddTag(Tag{n, n}) + assert.False(t, tags.Equals(other)) +} + +func TestTagEquals(t *testing.T) { + a, b, c := []byte("a"), []byte("b"), []byte("c") + assert.True(t, Tag{a, b}.Equals(Tag{a, b})) + assert.False(t, Tag{a, b}.Equals(Tag{a, c})) + assert.False(t, Tag{a, b}.Equals(Tag{b, c})) + assert.False(t, Tag{a, b}.Equals(Tag{b, b})) +} + func TestTagAppend(t *testing.T) { tagsToAdd := Tags{ Tags: []Tag{ diff --git a/src/query/models/types.go b/src/query/models/types.go index 913599fd3f..64558d6852 100644 --- a/src/query/models/types.go +++ b/src/query/models/types.go @@ -97,6 +97,8 @@ type TagOptions interface { SetIDSchemeType(scheme IDSchemeType) TagOptions // IDSchemeType gets the ID generation scheme type. IDSchemeType() IDSchemeType + // Equals determines if two tag options are equivalent. + Equals(other TagOptions) bool } // Tags represents a set of tags with options. diff --git a/src/query/test/block.go b/src/query/test/block.go index c4ae1a18f2..0245d49bdf 100644 --- a/src/query/test/block.go +++ b/src/query/test/block.go @@ -185,14 +185,20 @@ func NewBlockFromValuesWithMetaAndSeriesMeta( seriesMeta, ) - if err := columnBuilder.AddCols(len(seriesValues[0])); err != nil { + if err := columnBuilder.AddCols(meta.Bounds.Steps()); err != nil { panic(err) } - for _, seriesVal := range seriesValues { - for idx, val := range seriesVal { - if err := columnBuilder.AppendValue(idx, val); err != nil { - panic(err) + if len(seriesValues) > 0 { + for _, seriesVal := range seriesValues { + if meta.Bounds.Steps() != len(seriesVal) { + panic("invalid bounds for test series") + } + + for idx, val := range seriesVal { + if err := columnBuilder.AppendValue(idx, val); err != nil { + panic(err) + } } } } @@ -228,3 +234,21 @@ func GenerateValuesAndBounds( return values, bounds } + +// MustMakeTags creates tags given that the number of args is even. +func MustMakeTags(tag ...string) models.Tags { + if len(tag)%2 != 0 { + panic("must have even tag length") + } + + tagLength := len(tag) / 2 + t := models.NewTags(tagLength, models.NewTagOptions()) + for i := 0; i < tagLength; i++ { + t = t.AddTag(models.Tag{ + Name: []byte(tag[i*2]), + Value: []byte(tag[i*2+1]), + }) + } + + return t +} From cff085225b2ca62f223f8a6bab6a85999a573378 Mon Sep 17 00:00:00 2001 From: Artem Nikolayevsky Date: Wed, 14 Aug 2019 19:53:51 -0700 Subject: [PATCH 3/6] pr response --- src/query/block/block_test.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/query/block/block_test.go b/src/query/block/block_test.go index 26ee048103..9a7962a437 100644 --- a/src/query/block/block_test.go +++ b/src/query/block/block_test.go @@ -21,7 +21,6 @@ package block import ( - "fmt" "testing" "github.com/m3db/m3/src/query/models" @@ -30,24 +29,6 @@ import ( "github.com/stretchr/testify/require" ) -// MustMakeTags creates tags given that the number of args is even. -func MustMakeTags(tag ...string) models.Tags { - if len(tag)%2 != 0 { - panic("must have even tag length") - } - - tagLength := len(tag) / 2 - t := models.NewTags(tagLength, models.NewTagOptions()) - for i := 0; i < tagLength; i++ { - t = t.AddTag(models.Tag{ - Name: []byte(tag[i*2]), - Value: []byte(tag[i*2+1]), - }) - } - - return t -} - // MustMakeMeta creates metadata with given bounds and tags provided the number // is even. func MustMakeMeta(bounds models.Bounds, tags ...string) Metadata { @@ -70,8 +51,6 @@ func CompareMeta(t *testing.T, ex, ac Metadata) { actualTags := ac.Tags.Tags require.Equal(t, len(expectedTags), len(actualTags)) for i, tag := range expectedTags { - fmt.Println("x", string(tag.Name), ":", string(tag.Value)) - fmt.Println("a", string(actualTags[i].Name), ":", string(actualTags[i].Value)) assert.Equal(t, string(tag.Name), string(actualTags[i].Name)) assert.Equal(t, string(tag.Value), string(actualTags[i].Value)) } From aec7c6b038413115626f011148678614a649e703 Mon Sep 17 00:00:00 2001 From: Artem Nikolayevsky Date: Mon, 19 Aug 2019 18:34:23 -0400 Subject: [PATCH 4/6] fix test break --- src/query/block/block_test.go | 19 ------------------- src/query/block/empty_test.go | 22 +++++++++++++++++++++- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/query/block/block_test.go b/src/query/block/block_test.go index 9a7962a437..c43ddd5733 100644 --- a/src/query/block/block_test.go +++ b/src/query/block/block_test.go @@ -23,29 +23,10 @@ package block import ( "testing" - "github.com/m3db/m3/src/query/models" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// MustMakeMeta creates metadata with given bounds and tags provided the number -// is even. -func MustMakeMeta(bounds models.Bounds, tags ...string) Metadata { - return Metadata{ - Tags: MustMakeTags(tags...), - Bounds: bounds, - } -} - -// MustMakeSeriesMeta creates series metadata with given bounds and tags -// provided the number is even. -func MustMakeSeriesMeta(tags ...string) SeriesMeta { - return SeriesMeta{ - Tags: MustMakeTags(tags...), - } -} - func CompareMeta(t *testing.T, ex, ac Metadata) { expectedTags := ex.Tags.Tags actualTags := ac.Tags.Tags diff --git a/src/query/block/empty_test.go b/src/query/block/empty_test.go index 5857f066ab..e630470e40 100644 --- a/src/query/block/empty_test.go +++ b/src/query/block/empty_test.go @@ -40,8 +40,28 @@ var ( } ) +func mustMakeTags(tag ...string) models.Tags { + if len(tag)%2 != 0 { + panic("must have even tag length") + } + + tagLength := len(tag) / 2 + t := models.NewTags(tagLength, models.NewTagOptions()) + for i := 0; i < tagLength; i++ { + t = t.AddTag(models.Tag{ + Name: []byte(tag[i*2]), + Value: []byte(tag[i*2+1]), + }) + } + + return t +} + func mustMakeMeta(tags ...string) Metadata { - return MustMakeMeta(testBound, tags...) + return Metadata{ + Tags: mustMakeTags(tags...), + Bounds: testBound, + } } func TestEmptyBlock(t *testing.T) { From dc4fd0e82aada1c0f0c935fd6271786cc22d9a08 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Thu, 22 Aug 2019 13:55:02 -0400 Subject: [PATCH 5/6] Make test functions reused instead of duplicated and fix comment --- src/query/block/empty_test.go | 29 ++--------- src/query/block/scalar.go | 2 +- src/query/block/test/test.go | 42 ++++++++++++++++ .../functions/aggregation/absent_test.go | 50 +++++++------------ src/query/models/tags.go | 4 -- src/query/models/test/tags.go | 41 +++++++++++++++ src/query/test/block.go | 18 ------- 7 files changed, 106 insertions(+), 80 deletions(-) create mode 100644 src/query/block/test/test.go create mode 100644 src/query/models/test/tags.go diff --git a/src/query/block/empty_test.go b/src/query/block/empty_test.go index e630470e40..b693f92011 100644 --- a/src/query/block/empty_test.go +++ b/src/query/block/empty_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + "github.com/m3db/m3/src/query/block/test" "github.com/m3db/m3/src/query/models" "github.com/stretchr/testify/assert" @@ -40,32 +41,8 @@ var ( } ) -func mustMakeTags(tag ...string) models.Tags { - if len(tag)%2 != 0 { - panic("must have even tag length") - } - - tagLength := len(tag) / 2 - t := models.NewTags(tagLength, models.NewTagOptions()) - for i := 0; i < tagLength; i++ { - t = t.AddTag(models.Tag{ - Name: []byte(tag[i*2]), - Value: []byte(tag[i*2+1]), - }) - } - - return t -} - -func mustMakeMeta(tags ...string) Metadata { - return Metadata{ - Tags: mustMakeTags(tags...), - Bounds: testBound, - } -} - func TestEmptyBlock(t *testing.T) { - meta := mustMakeMeta("a", "b") + meta := test.MustMakeMeta(testBound, "a", "b") bl := NewEmptyBlock(meta) series, err := bl.SeriesIter() @@ -93,7 +70,7 @@ func TestEmptyBlock(t *testing.T) { } func TestEmptyUnconsolidatedBlock(t *testing.T) { - meta := mustMakeMeta("a", "b") + meta := test.MustMakeMeta(testBound, "a", "b") b := NewEmptyBlock(meta) bl, err := b.Unconsolidated() assert.NoError(t, err) diff --git a/src/query/block/scalar.go b/src/query/block/scalar.go index 0f456f5bfd..d2c8ee1678 100644 --- a/src/query/block/scalar.go +++ b/src/query/block/scalar.go @@ -29,7 +29,7 @@ import ( // Scalar is a block containing a single value over a certain bound // This represents constant values; it greatly simplifies downstream operations -// vy allowing them to treat this as a regular block, while at the same time +// by allowing them to treat this as a regular block, while at the same time // having an option to optimize by accessing the scalar value directly instead. type Scalar struct { s ScalarFunc diff --git a/src/query/block/test/test.go b/src/query/block/test/test.go new file mode 100644 index 0000000000..3559307ae5 --- /dev/null +++ b/src/query/block/test/test.go @@ -0,0 +1,42 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package test + +import ( + "github.com/m3db/m3/src/query/block" + "github.com/m3db/m3/src/query/models" + "github.com/m3db/m3/src/query/models/test" +) + +// MustMakeMeta returns block metadata or panics. +func MustMakeMeta(bounds models.Bounds, tags ...string) block.Metadata { + return block.Metadata{ + Tags: test.MustMakeTags(tags...), + Bounds: bounds, + } +} + +// MustMakeSeriesMeta returns series metadata or panics. +func MustMakeSeriesMeta(tags ...string) block.SeriesMeta { + return block.SeriesMeta{ + Tags: test.MustMakeTags(tags...), + } +} diff --git a/src/query/functions/aggregation/absent_test.go b/src/query/functions/aggregation/absent_test.go index af73e21296..385c5ca379 100644 --- a/src/query/functions/aggregation/absent_test.go +++ b/src/query/functions/aggregation/absent_test.go @@ -26,6 +26,7 @@ import ( "time" "github.com/m3db/m3/src/query/block" + blocktest "github.com/m3db/m3/src/query/block/test" "github.com/m3db/m3/src/query/executor/transform" "github.com/m3db/m3/src/query/models" "github.com/m3db/m3/src/query/parser" @@ -47,19 +48,6 @@ var ( } ) -func mustMakeMeta(tags ...string) block.Metadata { - return block.Metadata{ - Bounds: testBound, - Tags: test.MustMakeTags(tags...), - } -} - -func mustMakeSeriesMeta(tags ...string) block.SeriesMeta { - return block.SeriesMeta{ - Tags: test.MustMakeTags(tags...), - } -} - var absentTests = []struct { name string meta block.Metadata @@ -70,62 +58,62 @@ var absentTests = []struct { }{ { "no series", - mustMakeMeta(), + blocktest.MustMakeMeta(testBound), []block.SeriesMeta{}, [][]float64{}, - mustMakeMeta(), + blocktest.MustMakeMeta(testBound), []float64{1, 1, 1, 1}, }, { "no series with tags", - mustMakeMeta("A", "B", "C", "D"), + blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), []block.SeriesMeta{}, [][]float64{}, - mustMakeMeta("A", "B", "C", "D"), + blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), []float64{1, 1, 1, 1}, }, { "series with tags and values", - mustMakeMeta("A", "B", "C", "D"), - []block.SeriesMeta{mustMakeSeriesMeta("B", "B")}, + blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), + []block.SeriesMeta{blocktest.MustMakeSeriesMeta("B", "B")}, [][]float64{{1, 1, 1, 1}}, - mustMakeMeta("A", "B", "B", "B", "C", "D"), + blocktest.MustMakeMeta(testBound, "A", "B", "B", "B", "C", "D"), nil, }, { "series with tags and some missing", - mustMakeMeta("A", "B", "C", "D"), - []block.SeriesMeta{mustMakeSeriesMeta("bar", "baz")}, + blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), + []block.SeriesMeta{blocktest.MustMakeSeriesMeta("bar", "baz")}, [][]float64{{1, 1, 1, math.NaN()}}, - mustMakeMeta("A", "B", "bar", "baz", "C", "D"), + blocktest.MustMakeMeta(testBound, "A", "B", "bar", "baz", "C", "D"), []float64{nan, nan, nan, 1}, }, { "series with mismatched tags", - mustMakeMeta("A", "B", "C", "D"), + blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), []block.SeriesMeta{ - mustMakeSeriesMeta("B", "B"), - mustMakeSeriesMeta("F", "F"), + blocktest.MustMakeSeriesMeta("B", "B"), + blocktest.MustMakeSeriesMeta("F", "F"), }, [][]float64{ {1, 1, 1, math.NaN()}, {math.NaN(), 1, 1, math.NaN()}, }, - mustMakeMeta("A", "B", "C", "D"), + blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), []float64{nan, nan, nan, 1}, }, { "series with no missing values", - mustMakeMeta("A", "B", "C", "D"), + blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), []block.SeriesMeta{ - mustMakeSeriesMeta("F", "F"), - mustMakeSeriesMeta("F", "F"), + blocktest.MustMakeSeriesMeta("F", "F"), + blocktest.MustMakeSeriesMeta("F", "F"), }, [][]float64{ {1, math.NaN(), math.NaN(), 2}, {math.NaN(), 1, 1, math.NaN()}, }, - mustMakeMeta("A", "B", "C", "D", "F", "F"), + blocktest.MustMakeMeta(testBound, "A", "B", "C", "D", "F", "F"), nil, }, } diff --git a/src/query/models/tags.go b/src/query/models/tags.go index fd9db38ff5..a629381129 100644 --- a/src/query/models/tags.go +++ b/src/query/models/tags.go @@ -469,17 +469,13 @@ var tagSeperator = []byte(", ") // String returns the string representation of the tags. func (t Tags) String() string { var sb strings.Builder - // tags := make([]string, len(t.Tags)) for i, tt := range t.Tags { if i != 0 { sb.Write(tagSeperator) } - sb.WriteString(tt.String()) } - return sb.String() - // return strings.Join(tags, ", ") } // String returns the string representation of the tag. diff --git a/src/query/models/test/tags.go b/src/query/models/test/tags.go new file mode 100644 index 0000000000..9d38823d2b --- /dev/null +++ b/src/query/models/test/tags.go @@ -0,0 +1,41 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package test + +import "github.com/m3db/m3/src/query/models" + +// MustMakeTags creates tags given that the number of args is even. +func MustMakeTags(tag ...string) models.Tags { + if len(tag)%2 != 0 { + panic("must have even tag length") + } + + tagLength := len(tag) / 2 + t := models.NewTags(tagLength, models.NewTagOptions()) + for i := 0; i < tagLength; i++ { + t = t.AddTag(models.Tag{ + Name: []byte(tag[i*2]), + Value: []byte(tag[i*2+1]), + }) + } + + return t +} diff --git a/src/query/test/block.go b/src/query/test/block.go index 0245d49bdf..93bc4dc0c1 100644 --- a/src/query/test/block.go +++ b/src/query/test/block.go @@ -234,21 +234,3 @@ func GenerateValuesAndBounds( return values, bounds } - -// MustMakeTags creates tags given that the number of args is even. -func MustMakeTags(tag ...string) models.Tags { - if len(tag)%2 != 0 { - panic("must have even tag length") - } - - tagLength := len(tag) / 2 - t := models.NewTags(tagLength, models.NewTagOptions()) - for i := 0; i < tagLength; i++ { - t = t.AddTag(models.Tag{ - Name: []byte(tag[i*2]), - Value: []byte(tag[i*2+1]), - }) - } - - return t -} From 1f25ad9dac1152c8da1751a419bb9f7663e25623 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Thu, 22 Aug 2019 14:17:30 -0400 Subject: [PATCH 6/6] Move Must... prefixed methods --- src/query/block/empty_test.go | 5 +-- src/query/block/{test/test.go => unsafe.go} | 20 +++++----- .../functions/aggregation/absent_test.go | 37 +++++++++---------- src/query/models/{test/tags.go => unsafe.go} | 10 ++--- 4 files changed, 33 insertions(+), 39 deletions(-) rename src/query/block/{test/test.go => unsafe.go} (72%) rename src/query/models/{test/tags.go => unsafe.go} (88%) diff --git a/src/query/block/empty_test.go b/src/query/block/empty_test.go index b693f92011..8e949bb9d4 100644 --- a/src/query/block/empty_test.go +++ b/src/query/block/empty_test.go @@ -24,7 +24,6 @@ import ( "testing" "time" - "github.com/m3db/m3/src/query/block/test" "github.com/m3db/m3/src/query/models" "github.com/stretchr/testify/assert" @@ -42,7 +41,7 @@ var ( ) func TestEmptyBlock(t *testing.T) { - meta := test.MustMakeMeta(testBound, "a", "b") + meta := MustMakeMeta(testBound, "a", "b") bl := NewEmptyBlock(meta) series, err := bl.SeriesIter() @@ -70,7 +69,7 @@ func TestEmptyBlock(t *testing.T) { } func TestEmptyUnconsolidatedBlock(t *testing.T) { - meta := test.MustMakeMeta(testBound, "a", "b") + meta := MustMakeMeta(testBound, "a", "b") b := NewEmptyBlock(meta) bl, err := b.Unconsolidated() assert.NoError(t, err) diff --git a/src/query/block/test/test.go b/src/query/block/unsafe.go similarity index 72% rename from src/query/block/test/test.go rename to src/query/block/unsafe.go index 3559307ae5..09aaeb0323 100644 --- a/src/query/block/test/test.go +++ b/src/query/block/unsafe.go @@ -18,25 +18,23 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -package test +package block import ( - "github.com/m3db/m3/src/query/block" "github.com/m3db/m3/src/query/models" - "github.com/m3db/m3/src/query/models/test" ) -// MustMakeMeta returns block metadata or panics. -func MustMakeMeta(bounds models.Bounds, tags ...string) block.Metadata { - return block.Metadata{ - Tags: test.MustMakeTags(tags...), +// MustMakeMeta returns block metadata or panics (unsafe for use). +func MustMakeMeta(bounds models.Bounds, tags ...string) Metadata { + return Metadata{ + Tags: models.MustMakeTags(tags...), Bounds: bounds, } } -// MustMakeSeriesMeta returns series metadata or panics. -func MustMakeSeriesMeta(tags ...string) block.SeriesMeta { - return block.SeriesMeta{ - Tags: test.MustMakeTags(tags...), +// MustMakeSeriesMeta returns series metadata or panics (unsafe for use). +func MustMakeSeriesMeta(tags ...string) SeriesMeta { + return SeriesMeta{ + Tags: models.MustMakeTags(tags...), } } diff --git a/src/query/functions/aggregation/absent_test.go b/src/query/functions/aggregation/absent_test.go index 385c5ca379..1e4fa52d1a 100644 --- a/src/query/functions/aggregation/absent_test.go +++ b/src/query/functions/aggregation/absent_test.go @@ -26,7 +26,6 @@ import ( "time" "github.com/m3db/m3/src/query/block" - blocktest "github.com/m3db/m3/src/query/block/test" "github.com/m3db/m3/src/query/executor/transform" "github.com/m3db/m3/src/query/models" "github.com/m3db/m3/src/query/parser" @@ -58,62 +57,62 @@ var absentTests = []struct { }{ { "no series", - blocktest.MustMakeMeta(testBound), + block.MustMakeMeta(testBound), []block.SeriesMeta{}, [][]float64{}, - blocktest.MustMakeMeta(testBound), + block.MustMakeMeta(testBound), []float64{1, 1, 1, 1}, }, { "no series with tags", - blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), + block.MustMakeMeta(testBound, "A", "B", "C", "D"), []block.SeriesMeta{}, [][]float64{}, - blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), + block.MustMakeMeta(testBound, "A", "B", "C", "D"), []float64{1, 1, 1, 1}, }, { "series with tags and values", - blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), - []block.SeriesMeta{blocktest.MustMakeSeriesMeta("B", "B")}, + block.MustMakeMeta(testBound, "A", "B", "C", "D"), + []block.SeriesMeta{block.MustMakeSeriesMeta("B", "B")}, [][]float64{{1, 1, 1, 1}}, - blocktest.MustMakeMeta(testBound, "A", "B", "B", "B", "C", "D"), + block.MustMakeMeta(testBound, "A", "B", "B", "B", "C", "D"), nil, }, { "series with tags and some missing", - blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), - []block.SeriesMeta{blocktest.MustMakeSeriesMeta("bar", "baz")}, + block.MustMakeMeta(testBound, "A", "B", "C", "D"), + []block.SeriesMeta{block.MustMakeSeriesMeta("bar", "baz")}, [][]float64{{1, 1, 1, math.NaN()}}, - blocktest.MustMakeMeta(testBound, "A", "B", "bar", "baz", "C", "D"), + block.MustMakeMeta(testBound, "A", "B", "bar", "baz", "C", "D"), []float64{nan, nan, nan, 1}, }, { "series with mismatched tags", - blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), + block.MustMakeMeta(testBound, "A", "B", "C", "D"), []block.SeriesMeta{ - blocktest.MustMakeSeriesMeta("B", "B"), - blocktest.MustMakeSeriesMeta("F", "F"), + block.MustMakeSeriesMeta("B", "B"), + block.MustMakeSeriesMeta("F", "F"), }, [][]float64{ {1, 1, 1, math.NaN()}, {math.NaN(), 1, 1, math.NaN()}, }, - blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), + block.MustMakeMeta(testBound, "A", "B", "C", "D"), []float64{nan, nan, nan, 1}, }, { "series with no missing values", - blocktest.MustMakeMeta(testBound, "A", "B", "C", "D"), + block.MustMakeMeta(testBound, "A", "B", "C", "D"), []block.SeriesMeta{ - blocktest.MustMakeSeriesMeta("F", "F"), - blocktest.MustMakeSeriesMeta("F", "F"), + block.MustMakeSeriesMeta("F", "F"), + block.MustMakeSeriesMeta("F", "F"), }, [][]float64{ {1, math.NaN(), math.NaN(), 2}, {math.NaN(), 1, 1, math.NaN()}, }, - blocktest.MustMakeMeta(testBound, "A", "B", "C", "D", "F", "F"), + block.MustMakeMeta(testBound, "A", "B", "C", "D", "F", "F"), nil, }, } diff --git a/src/query/models/test/tags.go b/src/query/models/unsafe.go similarity index 88% rename from src/query/models/test/tags.go rename to src/query/models/unsafe.go index 9d38823d2b..387f4399c3 100644 --- a/src/query/models/test/tags.go +++ b/src/query/models/unsafe.go @@ -18,20 +18,18 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -package test - -import "github.com/m3db/m3/src/query/models" +package models // MustMakeTags creates tags given that the number of args is even. -func MustMakeTags(tag ...string) models.Tags { +func MustMakeTags(tag ...string) Tags { if len(tag)%2 != 0 { panic("must have even tag length") } tagLength := len(tag) / 2 - t := models.NewTags(tagLength, models.NewTagOptions()) + t := NewTags(tagLength, NewTagOptions()) for i := 0; i < tagLength; i++ { - t = t.AddTag(models.Tag{ + t = t.AddTag(Tag{ Name: []byte(tag[i*2]), Value: []byte(tag[i*2+1]), })