From 5a9553b2c4fe404d2b9118ba7a93e69eb7b85c36 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Mon, 18 Sep 2017 12:33:34 -0500 Subject: [PATCH] Remove unused casting code from the query engine Originally, casting was performed inside of the query engine especially for call iterators. Currently, the engine takes care of all casting so we just need to normalize the iterators types for type safety reasons rather than actual functional reasons. Removing this code. Cover coverage showed that it was not hit when run against the actual server. I ran the tests package and got code coverage of the query package while running the tests in that package. --- query/internal/internal.pb.go | 2 +- query/iterator.gen.go | 8 ------ query/iterator.gen.go.tmpl | 4 --- query/iterator.go | 51 +++++++++++++++-------------------- query/iterator_test.go | 38 ++++++++++---------------- 5 files changed, 37 insertions(+), 66 deletions(-) diff --git a/query/internal/internal.pb.go b/query/internal/internal.pb.go index 22b31c85fb..cbf9b82bb6 100644 --- a/query/internal/internal.pb.go +++ b/query/internal/internal.pb.go @@ -538,7 +538,7 @@ func init() { proto.RegisterFile("internal/internal.proto", fileDescriptorIntern var fileDescriptorInternal = []byte{ // 769 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x09, 0x6e, 0x88, 0x02, 0xff, 0x8c, 0x55, 0xe1, 0x8e, 0xe3, 0x34, + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x8c, 0x55, 0xe1, 0x8e, 0xe3, 0x34, 0x10, 0x56, 0x92, 0x4d, 0xb7, 0x71, 0xb7, 0xf4, 0x30, 0x65, 0xb1, 0xd0, 0x09, 0x45, 0x11, 0x48, 0x11, 0xa0, 0x22, 0xed, 0x2f, 0x7e, 0x21, 0xf5, 0xd8, 0x5b, 0x54, 0xe9, 0xae, 0x7b, 0x72, 0x97, 0xfd, 0x6f, 0x9a, 0xd9, 0xc8, 0x52, 0xea, 0x14, 0xc7, 0x41, 0xed, 0x03, 0xf0, 0x10, 0x3c, 0x16, diff --git a/query/iterator.gen.go b/query/iterator.gen.go index e580177322..b779fde594 100644 --- a/query/iterator.gen.go +++ b/query/iterator.gen.go @@ -38,10 +38,6 @@ func newFloatIterators(itrs []Iterator) []FloatIterator { switch itr := itr.(type) { case FloatIterator: a = append(a, itr) - - case IntegerIterator: - a = append(a, &integerFloatCastIterator{input: itr}) - default: itr.Close() } @@ -3432,7 +3428,6 @@ func newIntegerIterators(itrs []Iterator) []IntegerIterator { switch itr := itr.(type) { case IntegerIterator: a = append(a, itr) - default: itr.Close() } @@ -6820,7 +6815,6 @@ func newUnsignedIterators(itrs []Iterator) []UnsignedIterator { switch itr := itr.(type) { case UnsignedIterator: a = append(a, itr) - default: itr.Close() } @@ -10194,7 +10188,6 @@ func newStringIterators(itrs []Iterator) []StringIterator { switch itr := itr.(type) { case StringIterator: a = append(a, itr) - default: itr.Close() } @@ -13568,7 +13561,6 @@ func newBooleanIterators(itrs []Iterator) []BooleanIterator { switch itr := itr.(type) { case BooleanIterator: a = append(a, itr) - default: itr.Close() } diff --git a/query/iterator.gen.go.tmpl b/query/iterator.gen.go.tmpl index 701efde865..7e9d7d47a2 100644 --- a/query/iterator.gen.go.tmpl +++ b/query/iterator.gen.go.tmpl @@ -34,10 +34,6 @@ func new{{$k.Name}}Iterators(itrs []Iterator) []{{$k.Name}}Iterator { switch itr := itr.(type) { case {{$k.Name}}Iterator: a = append(a, itr) -{{if eq .Name "Float"}} - case IntegerIterator: - a = append(a, &integerFloatCastIterator{input: itr}) -{{end}} default: itr.Close() } diff --git a/query/iterator.go b/query/iterator.go index 8520adbd0c..494926c475 100644 --- a/query/iterator.go +++ b/query/iterator.go @@ -61,40 +61,33 @@ func (a Iterators) filterNonNil() []Iterator { return other } -// castType determines what type to cast the set of iterators to. -// An iterator type is chosen using this hierarchy: -// float > integer > string > boolean -func (a Iterators) castType() influxql.DataType { +// dataType determines what slice type this set of iterators should be. +// An iterator type is chosen by looking at the first element in the slice +// and then returning the data type for that iterator. +func (a Iterators) dataType() influxql.DataType { if len(a) == 0 { return influxql.Unknown } - typ := influxql.DataType(influxql.Boolean) - for _, input := range a { - switch input.(type) { - case FloatIterator: - // Once a float iterator is found, short circuit the end. - return influxql.Float - case IntegerIterator: - if typ > influxql.Integer { - typ = influxql.Integer - } - case StringIterator: - if typ > influxql.String { - typ = influxql.String - } - case BooleanIterator: - // Boolean is the lowest type. - } + switch a[0].(type) { + case FloatIterator: + return influxql.Float + case IntegerIterator: + return influxql.Integer + case StringIterator: + return influxql.String + case BooleanIterator: + return influxql.Boolean + default: + return influxql.Unknown } - return typ } -// cast casts an array of iterators to a single type. -// Iterators that are not compatible or cannot be cast to the -// chosen iterator type are closed and dropped. -func (a Iterators) cast() interface{} { - typ := a.castType() +// coerce forces an array of iterators to be a single type. +// Iterators that are not of the same type as the first element in the slice +// will be closed and dropped. +func (a Iterators) coerce() interface{} { + typ := a.dataType() switch typ { case influxql.Float: return newFloatIterators(a) @@ -163,7 +156,7 @@ func NewMergeIterator(inputs []Iterator, opt IteratorOptions) Iterator { // Aggregate functions can use a more relaxed sorting so that points // within a window are grouped. This is much more efficient. - switch inputs := Iterators(inputs).cast().(type) { + switch inputs := Iterators(inputs).coerce().(type) { case []FloatIterator: return newFloatMergeIterator(inputs, opt) case []IntegerIterator: @@ -225,7 +218,7 @@ func NewSortedMergeIterator(inputs []Iterator, opt IteratorOptions) Iterator { return inputs[0] } - switch inputs := Iterators(inputs).cast().(type) { + switch inputs := Iterators(inputs).coerce().(type) { case []FloatIterator: return newFloatSortedMergeIterator(inputs, opt) case []IntegerIterator: diff --git a/query/iterator_test.go b/query/iterator_test.go index e4600c0a92..151c750bda 100644 --- a/query/iterator_test.go +++ b/query/iterator_test.go @@ -219,8 +219,14 @@ func TestMergeIterator_Nil(t *testing.T) { } } -func TestMergeIterator_Cast_Float(t *testing.T) { +func TestMergeIterator_Coerce_Float(t *testing.T) { inputs := []query.Iterator{ + &FloatIterator{Points: []query.FloatPoint{ + {Name: "cpu", Tags: ParseTags("host=A"), Time: 20, Value: 7}, + {Name: "cpu", Tags: ParseTags("host=B"), Time: 11, Value: 5}, + {Name: "cpu", Tags: ParseTags("host=B"), Time: 13, Value: 6}, + {Name: "mem", Tags: ParseTags("host=A"), Time: 25, Value: 9}, + }}, &IntegerIterator{Points: []query.IntegerPoint{ {Name: "cpu", Tags: ParseTags("host=A"), Time: 0, Value: 1}, {Name: "cpu", Tags: ParseTags("host=A"), Time: 12, Value: 3}, @@ -228,12 +234,6 @@ func TestMergeIterator_Cast_Float(t *testing.T) { {Name: "cpu", Tags: ParseTags("host=B"), Time: 1, Value: 2}, {Name: "mem", Tags: ParseTags("host=B"), Time: 11, Value: 8}, }}, - &FloatIterator{Points: []query.FloatPoint{ - {Name: "cpu", Tags: ParseTags("host=A"), Time: 20, Value: 7}, - {Name: "cpu", Tags: ParseTags("host=B"), Time: 11, Value: 5}, - {Name: "cpu", Tags: ParseTags("host=B"), Time: 13, Value: 6}, - {Name: "mem", Tags: ParseTags("host=A"), Time: 25, Value: 9}, - }}, } itr := query.NewMergeIterator(inputs, query.IteratorOptions{ @@ -246,15 +246,10 @@ func TestMergeIterator_Cast_Float(t *testing.T) { if a, err := Iterators([]query.Iterator{itr}).ReadAll(); err != nil { t.Fatalf("unexpected error: %s", err) } else if !deep.Equal(a, [][]query.Point{ - {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 0, Value: 1}}, - {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 12, Value: 3}}, {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 20, Value: 7}}, - {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 30, Value: 4}}, - {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=B"), Time: 1, Value: 2}}, {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=B"), Time: 11, Value: 5}}, {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=B"), Time: 13, Value: 6}}, {&query.FloatPoint{Name: "mem", Tags: ParseTags("host=A"), Time: 25, Value: 9}}, - {&query.FloatPoint{Name: "mem", Tags: ParseTags("host=B"), Time: 11, Value: 8}}, }) { t.Errorf("unexpected points: %s", spew.Sdump(a)) } @@ -472,8 +467,14 @@ func TestSortedMergeIterator_Nil(t *testing.T) { } } -func TestSortedMergeIterator_Cast_Float(t *testing.T) { +func TestSortedMergeIterator_Coerce_Float(t *testing.T) { inputs := []query.Iterator{ + &FloatIterator{Points: []query.FloatPoint{ + {Name: "cpu", Tags: ParseTags("host=A"), Time: 20, Value: 7}, + {Name: "cpu", Tags: ParseTags("host=B"), Time: 11, Value: 5}, + {Name: "cpu", Tags: ParseTags("host=B"), Time: 13, Value: 6}, + {Name: "mem", Tags: ParseTags("host=A"), Time: 25, Value: 9}, + }}, &IntegerIterator{Points: []query.IntegerPoint{ {Name: "cpu", Tags: ParseTags("host=A"), Time: 0, Value: 1}, {Name: "cpu", Tags: ParseTags("host=A"), Time: 12, Value: 3}, @@ -481,12 +482,6 @@ func TestSortedMergeIterator_Cast_Float(t *testing.T) { {Name: "cpu", Tags: ParseTags("host=B"), Time: 1, Value: 2}, {Name: "mem", Tags: ParseTags("host=B"), Time: 4, Value: 8}, }}, - &FloatIterator{Points: []query.FloatPoint{ - {Name: "cpu", Tags: ParseTags("host=A"), Time: 20, Value: 7}, - {Name: "cpu", Tags: ParseTags("host=B"), Time: 11, Value: 5}, - {Name: "cpu", Tags: ParseTags("host=B"), Time: 13, Value: 6}, - {Name: "mem", Tags: ParseTags("host=A"), Time: 25, Value: 9}, - }}, } itr := query.NewSortedMergeIterator(inputs, query.IteratorOptions{ @@ -499,15 +494,10 @@ func TestSortedMergeIterator_Cast_Float(t *testing.T) { if a, err := Iterators([]query.Iterator{itr}).ReadAll(); err != nil { t.Fatalf("unexpected error: %s", err) } else if !deep.Equal(a, [][]query.Point{ - {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 0, Value: 1}}, - {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 12, Value: 3}}, {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 20, Value: 7}}, - {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 30, Value: 4}}, - {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=B"), Time: 1, Value: 2}}, {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=B"), Time: 11, Value: 5}}, {&query.FloatPoint{Name: "cpu", Tags: ParseTags("host=B"), Time: 13, Value: 6}}, {&query.FloatPoint{Name: "mem", Tags: ParseTags("host=A"), Time: 25, Value: 9}}, - {&query.FloatPoint{Name: "mem", Tags: ParseTags("host=B"), Time: 4, Value: 8}}, }) { t.Errorf("unexpected points: %s", spew.Sdump(a)) }