Merge pull request influxdata#4993 from influxdb/nc-issue#4666

Validate previous value in derivative calls
zhang8h · Dec 4, 2015 · dd9d501 · dd9d501
2 parents 4f69936 + bd6961a
commit dd9d501
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 
 ### Bugfixes
 - [#4984](https://github.com/influxdb/influxdb/pull/4984): Allow math on fields, fixes regression. Thanks @mengjinglei
+- [#4666](https://github.com/influxdb/influxdb/issues/4666): Fix panic in derivative with invalid values.
 - [#4858](https://github.com/influxdb/influxdb/pull/4858): Validate nested aggregations in queries. Thanks @viru
 - [#4921](https://github.com/influxdb/influxdb/pull/4921): Error responses should be JSON-formatted. Thanks @pires
 - [#4974](https://github.com/influxdb/influxdb/issues/4974) Fix Data Race in TSDB when setting measurement field name

diff --git a/tsdb/raw.go b/tsdb/raw.go
@@ -702,15 +702,23 @@ func ProcessAggregateDerivative(results [][]interface{}, isNonNegative bool, int
 
 		// Check the value's type to ensure it's an numeric, if not, return a nil result. We only check the first value
 		// because derivatives cannot be combined with other aggregates currently.
-		validType := false
+		curValidType := false
 		switch cur[1].(type) {
 		case int64:
-			validType = true
+			curValidType = true
 		case float64:
-			validType = true
+			curValidType = true
 		}
 
-		if !validType {
+		prevValidType := false
+		switch prev[1].(type) {
+		case int64:
+			prevValidType = true
+		case float64:
+			prevValidType = true
+		}
+
+		if !curValidType || !prevValidType {
 			derivatives = append(derivatives, []interface{}{
 				cur[0], nil,
 			})

diff --git a/tsdb/raw_test.go b/tsdb/raw_test.go
@@ -798,6 +798,40 @@ func TestProcessAggregateDerivative_Bool(t *testing.T) {
 	}
 }
 
+func TestProcessAggregateDerivative_FirstInvalid(t *testing.T) {
+	results := tsdb.ProcessAggregateDerivative([][]interface{}{
+		[]interface{}{time.Unix(0, 0), "1.0"},
+		[]interface{}{time.Unix(0, 0).Add(24 * time.Hour), 2.0},
+		[]interface{}{time.Unix(0, 0).Add(48 * time.Hour), 3.0},
+		[]interface{}{time.Unix(0, 0).Add(72 * time.Hour), 4.0},
+	}, false, 24*time.Hour)
+
+	if !reflect.DeepEqual(results, [][]interface{}{
+		[]interface{}{time.Unix(0, 0).Add(24 * time.Hour), nil},
+		[]interface{}{time.Unix(0, 0).Add(48 * time.Hour), 1.0},
+		[]interface{}{time.Unix(0, 0).Add(72 * time.Hour), 1.0},
+	}) {
+		t.Fatalf("unexpected results: %s", spew.Sdump(results))
+	}
+}
+
+func TestProcessAggregateDerivative_RandomInvalid(t *testing.T) {
+	results := tsdb.ProcessAggregateDerivative([][]interface{}{
+		[]interface{}{time.Unix(0, 0), 1.0},
+		[]interface{}{time.Unix(0, 0).Add(24 * time.Hour), 2.0},
+		[]interface{}{time.Unix(0, 0).Add(48 * time.Hour), "3.0"},
+		[]interface{}{time.Unix(0, 0).Add(72 * time.Hour), 4.0},
+	}, false, 24*time.Hour)
+
+	if !reflect.DeepEqual(results, [][]interface{}{
+		[]interface{}{time.Unix(0, 0).Add(24 * time.Hour), 1.0},
+		[]interface{}{time.Unix(0, 0).Add(48 * time.Hour), nil},
+		[]interface{}{time.Unix(0, 0).Add(72 * time.Hour), nil},
+	}) {
+		t.Fatalf("unexpected results: %s", spew.Sdump(results))
+	}
+}
+
 func TestRawQueryDerivative_Process_Empty(t *testing.T) {
 	p := tsdb.RawQueryDerivativeProcessor{
 		IsNonNegative:      false,