Mean and percentile function fixes

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## v0.9.0-rc28 [unreleased]
 
+### Bugfixes
+- [#2374](https://github.com/influxdb/influxdb/issues/2374): Two different panics during SELECT percentile
+- [#2404](https://github.com/influxdb/influxdb/pull/2404): Mean and percentile function fixes
+
 ## v0.9.0-rc27 [04-23-2015]
 
 ### Features

cmd/influxd/server_integration_test.go

@@ -1456,9 +1456,7 @@ func TestSingleServerDiags(t *testing.T) {
 		t.Skip(fmt.Sprintf("skipping '%s'", testName))
 	}
 	dir := tempfile()
-	defer func() {
-		os.RemoveAll(dir)
-	}()
+	defer os.RemoveAll(dir)
 
 	config := main.NewConfig()
 	config.Monitoring.Enabled = true
@@ -1476,9 +1474,7 @@ func TestSingleServer(t *testing.T) {
 		t.Skip(fmt.Sprintf("skipping '%s'", testName))
 	}
 	dir := tempfile()
-	defer func() {
-		os.RemoveAll(dir)
-	}()
+	defer os.RemoveAll(dir)
 
 	nodes := createCombinedNodeCluster(t, testName, dir, 1, nil)
 	defer nodes.Close()
@@ -1495,9 +1491,7 @@ func Test3NodeServer(t *testing.T) {
 		t.Skip(fmt.Sprintf("skipping '%s'", testName))
 	}
 	dir := tempfile()
-	defer func() {
-		os.RemoveAll(dir)
-	}()
+	defer os.RemoveAll(dir)
 
 	nodes := createCombinedNodeCluster(t, testName, dir, 3, nil)
 	defer nodes.Close()
@@ -1514,9 +1508,7 @@ func Test3NodeServerFailover(t *testing.T) {
 		t.Skip(fmt.Sprintf("skipping '%s'", testName))
 	}
 	dir := tempfile()
-	defer func() {
-		os.RemoveAll(dir)
-	}()
+	defer os.RemoveAll(dir)
 
 	nodes := createCombinedNodeCluster(t, testName, dir, 3, nil)
 
@@ -1539,9 +1531,7 @@ func Test5NodeClusterPartiallyReplicated(t *testing.T) {
 		t.Skip(fmt.Sprintf("skipping '%s'", testName))
 	}
 	dir := tempfile()
-	defer func() {
-		os.RemoveAll(dir)
-	}()
+	defer os.RemoveAll(dir)
 
 	nodes := createCombinedNodeCluster(t, testName, dir, 5, nil)
 	defer nodes.Close()
@@ -1557,9 +1547,7 @@ func TestClientLibrary(t *testing.T) {
 		t.Skip(fmt.Sprintf("skipping '%s'", testName))
 	}
 	dir := tempfile()
-	defer func() {
-		os.RemoveAll(dir)
-	}()
+	defer os.RemoveAll(dir)
 
 	now := time.Now().UTC()
 

influxql/functions.go

@@ -107,6 +107,10 @@ func InitializeReduceFunc(c *Call) (ReduceFunc, error) {
 	case "last":
 		return ReduceLast, nil
 	case "percentile":
+		if len(c.Args) != 2 {
+			return nil, fmt.Errorf("expected float argument in percentile()")
+		}
+
 		lit, ok := c.Args[1].(*NumberLiteral)
 		if !ok {
 			return nil, fmt.Errorf("expected float argument in percentile()")
@@ -213,7 +217,12 @@ func MapMean(itr Iterator) interface{} {
 		out.Count++
 		out.Mean += (v.(float64) - out.Mean) / float64(out.Count)
 	}
-	return out
+
+	if out.Count > 0 {
+		return out
+	}
+
+	return nil
 }
 
 type meanMapOutput struct {
@@ -547,6 +556,10 @@ func ReducePercentile(percentile float64) ReduceFunc {
 		var allValues []float64
 
 		for _, v := range values {
+			if v == nil {
+				continue
+			}
+
 			vals := v.([]interface{})
 			for _, v := range vals {
 				allValues = append(allValues, v.(float64))

influxql/functions_test.go

@@ -0,0 +1,114 @@
+package influxql
+
+import "testing"
+
+type point struct {
+	seriesID  uint64
+	timestamp int64
+	value     interface{}
+}
+
+type testIterator struct {
+	values []point
+}
+
+func (t *testIterator) Next() (seriesID uint64, timestamp int64, value interface{}) {
+	if len(t.values) > 0 {
+		v := t.values[0]
+		t.values = t.values[1:]
+		return v.seriesID, v.timestamp, v.value
+	}
+	return 0, 0, nil
+}
+
+func TestMapMeanNoValues(t *testing.T) {
+	iter := &testIterator{}
+	if got := MapMean(iter); got != nil {
+		t.Errorf("output mismatch: exp nil got %v", got)
+	}
+}
+
+func TestMapMean(t *testing.T) {
+
+	tests := []struct {
+		input  []point
+		output *meanMapOutput
+	}{
+		{ // Single point
+			input: []point{
+				point{0, 1, 1.0},
+			},
+			output: &meanMapOutput{1, 1},
+		},
+		{ // Two points
+			input: []point{
+				point{0, 1, 2.0},
+				point{0, 2, 8.0},
+			},
+			output: &meanMapOutput{2, 5.0},
+		},
+	}
+
+	for _, test := range tests {
+		iter := &testIterator{
+			values: test.input,
+		}
+
+		got := MapMean(iter)
+		if got == nil {
+			t.Fatalf("MapMean(%v): output mismatch: exp %v got %v", test.input, test.output, got)
+		}
+
+		if got.(*meanMapOutput).Count != test.output.Count || got.(*meanMapOutput).Mean != test.output.Mean {
+			t.Errorf("output mismatch: exp %v got %v", test.output, got)
+		}
+
+	}
+}
+
+func TestInitializeReduceFuncPercentile(t *testing.T) {
+	// No args
+	c := &Call{
+		Name: "percentile",
+		Args: []Expr{},
+	}
+	_, err := InitializeReduceFunc(c)
+	if err == nil {
+		t.Errorf("InitializedReduceFunc(%v) expected error. got nil", c)
+	}
+
+	if exp := "expected float argument in percentile()"; err.Error() != exp {
+		t.Errorf("InitializedReduceFunc(%v) mismatch. exp %v got %v", c, exp, err.Error())
+	}
+
+	// No percentile arg
+	c = &Call{
+		Name: "percentile",
+		Args: []Expr{
+			&VarRef{Val: "field1"},
+		},
+	}
+
+	_, err = InitializeReduceFunc(c)
+	if err == nil {
+		t.Errorf("InitializedReduceFunc(%v) expected error. got nil", c)
+	}
+
+	if exp := "expected float argument in percentile()"; err.Error() != exp {
+		t.Errorf("InitializedReduceFunc(%v) mismatch. exp %v got %v", c, exp, err.Error())
+	}
+}
+
+func TestReducePercentileNil(t *testing.T) {
+
+	// ReducePercentile should ignore nil values when calculating the percentile
+	fn := ReducePercentile(100)
+	input := []interface{}{
+		nil,
+	}
+
+	got := fn(input)
+	if got != nil {
+		t.Fatalf("ReducePercentile(100) returned wrong type. exp nil got %v", got)
+	}
+}