Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add test for large mean aggregations #2390

Merged
merged 2 commits into from
Apr 22, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cmd/influxd/server_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/ioutil"
"log"
"math"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -397,6 +398,7 @@ func runTestsData(t *testing.T, testName string, nodes Cluster, database, retent

yesterday := time.Now().Add(-1 * time.Hour * 24).UTC()
now := time.Now().UTC()
maxFloat64, _ := json.Marshal(math.MaxFloat64)

// Start by ensuring database and retention policy exist.
createDatabase(t, testName, nodes, database)
Expand Down Expand Up @@ -568,6 +570,17 @@ func runTestsData(t *testing.T, testName string, nodes Cluster, database, retent
},

// Aggregations
{
reset: true,
name: "large mean",
write: `{"database" : "%DB%", "retentionPolicy" : "%RP%", "points": [
{"name": "cpu", "timestamp": "2015-04-20T14:27:40Z", "fields": {"value": ` + string(maxFloat64) + `}},
{"name": "cpu", "timestamp": "2015-04-20T14:27:41Z", "fields": {"value": ` + string(maxFloat64) + `}}
]}`,
query: `SELECT mean(value) FROM cpu`,
queryDb: "%DB%",
expected: `{"results":[{"series":[{"name":"cpu","columns":["time","mean"],"values":[["1970-01-01T00:00:00Z",` + string(maxFloat64) + `]]}]}]}`,
},
{
reset: true,
name: "aggregations",
Expand Down
12 changes: 7 additions & 5 deletions influxql/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,29 +211,31 @@ func MapMean(itr Iterator) interface{} {

for _, k, v := itr.Next(); k != 0; _, k, v = itr.Next() {
out.Count++
out.Sum += v.(float64)
out.Mean += (v.(float64) - out.Mean) / float64(out.Count)
}
return out
}

type meanMapOutput struct {
Count int
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toddboom i had a question about this... should Count be a float64? everywhere it gets used (except for where it's calculated) it gets cast to float64 and i don't know go well enough to know what is more idiomatic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think leaving count as an integer makes sense to me. I don't think the cast is particularly expensive, so I'd leave it that way for now unless you feel strongly otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't have a strong feeling - only uncertainty - so i'll leave it as is 😃

Sum float64
Mean float64
}

// ReduceMean computes the mean of values for each key.
func ReduceMean(values []interface{}) interface{} {
out := &meanMapOutput{}
var countSum int
for _, v := range values {
if v == nil {
continue
}
val := v.(*meanMapOutput)
out.Count += val.Count
out.Sum += val.Sum
countSum = out.Count + val.Count
out.Mean = val.Mean*(float64(val.Count)/float64(countSum)) + out.Mean*(float64(out.Count)/float64(countSum))
out.Count = countSum
}
if out.Count > 0 {
return out.Sum / float64(out.Count)
return out.Mean
}
return nil
}
Expand Down