From 2802047e68b1004ef393acd9ff723069bb4c0607 Mon Sep 17 00:00:00 2001 From: Henrik Johansson Date: Mon, 15 Jun 2015 16:09:50 +0200 Subject: [PATCH 1/5] Added failing test for #2956 --- influxql/ast.go | 2 +- influxql/engine_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/influxql/ast.go b/influxql/ast.go index 938e548c52..d67e203d43 100644 --- a/influxql/ast.go +++ b/influxql/ast.go @@ -949,7 +949,7 @@ func (s *SelectStatement) validateAggregates(tr targetRequirement) error { for _, f := range s.Fields { if c, ok := f.Expr.(*Call); ok { switch c.Name { - case "derivative", "non_negative_derivative": + case "derivative", "non_negative_derivative", "non_integer_derivative": if min, max, got := 1, 2, len(c.Args); got > max || got < min { return fmt.Errorf("invalid number of arguments for %s, expected at least %d but no more than %d, got %d", c.Name, min, max, got) } diff --git a/influxql/engine_test.go b/influxql/engine_test.go index fc9dd14fb6..708f92fc81 100644 --- a/influxql/engine_test.go +++ b/influxql/engine_test.go @@ -206,6 +206,36 @@ func TestProcessDerivative(t *testing.T) { }, }, }, + { + name: "float derivatives", + fn: "non_integer_derivative", + interval: "1d", + in: [][]interface{}{ + []interface{}{ + time.Unix(0, 0), 1.0, + }, + []interface{}{ + time.Unix(0, 0).Add(24 * time.Hour), int64(3), + }, + []interface{}{ + time.Unix(0, 0).Add(48 * time.Hour), int64(5), + }, + []interface{}{ + time.Unix(0, 0).Add(72 * time.Hour), int64(9), + }, + }, + exp: [][]interface{}{ + []interface{}{ + time.Unix(0, 0).Add(24 * time.Hour), 2.0, + }, + []interface{}{ + time.Unix(0, 0).Add(48 * time.Hour), 2.0, + }, + []interface{}{ + time.Unix(0, 0).Add(72 * time.Hour), 4.0, + }, + }, + }, } for _, test := range tests { From 41218f1a66727c4786a838004de02c0483667a75 Mon Sep 17 00:00:00 2001 From: Henrik Johansson Date: Mon, 15 Jun 2015 16:10:06 +0200 Subject: [PATCH 2/5] Fixed #2956 --- influxql/engine.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/influxql/engine.go b/influxql/engine.go index 7f4bb8c37e..d57ee14e5c 100644 --- a/influxql/engine.go +++ b/influxql/engine.go @@ -3,6 +3,7 @@ package influxql import ( "bytes" "errors" + "fmt" "hash/fnv" "math" "sort" @@ -408,7 +409,8 @@ func (m *MapReduceJob) processRawQueryDerivative(lastValueFromPreviousChunk *raw // Calculate the derivative of successive points by dividing the difference // of each value by the elapsed time normalized to the interval - diff := v.Values.(float64) - lastValueFromPreviousChunk.Values.(float64) + diff := fromInt64ToFloat64(v.Values) - fromInt64ToFloat64(lastValueFromPreviousChunk.Values) + elapsed := v.Time - lastValueFromPreviousChunk.Time value := 0.0 @@ -468,7 +470,7 @@ func (m *MapReduceJob) processDerivative(results [][]interface{}) [][]interface{ } elapsed := cur[0].(time.Time).Sub(prev[0].(time.Time)) - diff := cur[1].(float64) - prev[1].(float64) + diff := fromInt64ToFloat64(cur[1]) - fromInt64ToFloat64(prev[1]) value := 0.0 if elapsed > 0 { value = float64(diff) / (float64(elapsed) / float64(m.derivativeInterval())) @@ -930,6 +932,16 @@ func (e *Executor) execute(out chan *Row) { close(out) } +func fromInt64ToFloat64(v interface{}) float64 { + switch v.(type) { + case int64: + return float64(v.(int64)) + case float64: + return v.(float64) + } + panic(fmt.Sprintf("Expected either int64 or float64, got %v", v)) +} + // Row represents a single row returned from the execution of a statement. type Row struct { Name string `json:"name,omitempty"` From 956b716f50069dc0002aa72b20ee927babe14036 Mon Sep 17 00:00:00 2001 From: Henrik Johansson Date: Mon, 15 Jun 2015 16:19:53 +0200 Subject: [PATCH 3/5] Added CHANGELOG entry for #2956 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cab7e1683..3235bc723c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Bugfixes - [3013](https://github.com/influxdb/influxdb/issues/3013): Panic error with inserting values with commas +- [#2956](https://github.com/influxdb/influxdb/issues/2956): Type mismatch in derivative - [#2908](https://github.com/influxdb/influxdb/issues/2908): Field mismatch error messages need to be updated - [#2931](https://github.com/influxdb/influxdb/pull/2931): Services and reporting should wait until cluster has leader. - [#2943](https://github.com/influxdb/influxdb/issues/2943): Ensure default retention policies are fully replicated From 8bb2fc52a90248170608c87c3a3e4bc581907989 Mon Sep 17 00:00:00 2001 From: Henrik Johansson Date: Mon, 15 Jun 2015 17:33:46 +0200 Subject: [PATCH 4/5] Reverted the addition of extra function to the ast for #2956. Accidentally added due to ignorance really. --- influxql/ast.go | 2 +- influxql/engine_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/influxql/ast.go b/influxql/ast.go index d67e203d43..938e548c52 100644 --- a/influxql/ast.go +++ b/influxql/ast.go @@ -949,7 +949,7 @@ func (s *SelectStatement) validateAggregates(tr targetRequirement) error { for _, f := range s.Fields { if c, ok := f.Expr.(*Call); ok { switch c.Name { - case "derivative", "non_negative_derivative", "non_integer_derivative": + case "derivative", "non_negative_derivative": if min, max, got := 1, 2, len(c.Args); got > max || got < min { return fmt.Errorf("invalid number of arguments for %s, expected at least %d but no more than %d, got %d", c.Name, min, max, got) } diff --git a/influxql/engine_test.go b/influxql/engine_test.go index 708f92fc81..9cdd696c64 100644 --- a/influxql/engine_test.go +++ b/influxql/engine_test.go @@ -208,7 +208,7 @@ func TestProcessDerivative(t *testing.T) { }, { name: "float derivatives", - fn: "non_integer_derivative", + fn: "derivative", interval: "1d", in: [][]interface{}{ []interface{}{ From 0c59240e8098621642b987de934e7f53792f6def Mon Sep 17 00:00:00 2001 From: Henrik Johansson Date: Mon, 15 Jun 2015 17:45:42 +0200 Subject: [PATCH 5/5] Review fixes for #2956 --- influxql/engine.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/influxql/engine.go b/influxql/engine.go index d57ee14e5c..7ebdb57e18 100644 --- a/influxql/engine.go +++ b/influxql/engine.go @@ -409,7 +409,7 @@ func (m *MapReduceJob) processRawQueryDerivative(lastValueFromPreviousChunk *raw // Calculate the derivative of successive points by dividing the difference // of each value by the elapsed time normalized to the interval - diff := fromInt64ToFloat64(v.Values) - fromInt64ToFloat64(lastValueFromPreviousChunk.Values) + diff := i64tof64(v.Values) - i64tof64(lastValueFromPreviousChunk.Values) elapsed := v.Time - lastValueFromPreviousChunk.Time @@ -470,7 +470,7 @@ func (m *MapReduceJob) processDerivative(results [][]interface{}) [][]interface{ } elapsed := cur[0].(time.Time).Sub(prev[0].(time.Time)) - diff := fromInt64ToFloat64(cur[1]) - fromInt64ToFloat64(prev[1]) + diff := i64tof64(cur[1]) - i64tof64(prev[1]) value := 0.0 if elapsed > 0 { value = float64(diff) / (float64(elapsed) / float64(m.derivativeInterval())) @@ -932,14 +932,14 @@ func (e *Executor) execute(out chan *Row) { close(out) } -func fromInt64ToFloat64(v interface{}) float64 { +func i64tof64(v interface{}) float64 { switch v.(type) { case int64: return float64(v.(int64)) case float64: return v.(float64) } - panic(fmt.Sprintf("Expected either int64 or float64, got %v", v)) + panic(fmt.Sprintf("expected either int64 or float64, got %v", v)) } // Row represents a single row returned from the execution of a statement.