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

Handle missing values in aggregate derivative queries better #4292

Merged
merged 1 commit into from
Oct 1, 2015

Conversation

jwilder
Copy link
Contributor

@jwilder jwilder commented Oct 1, 2015

If an aggregate derivative query did not have a value in the first
time bucket, it would abort early and return a single row with a value
of 0. Similarly, if either the current or previous value was nil,
it would skip the row and not append any values causing gaps and
no data to show up.

Instead, this will append a nil value if either the current or previous
value is nil. This essentially allows nil values to carry through the
results as well as gives a more sensible value for rows where we cannot
compute a difference (instead of dropping the row as before).

Fixes #4237 #4263

@jwilder jwilder changed the title Handle missing values in aggregate derivative quries better Handle missing values in aggregate derivative queries better Oct 1, 2015
If an aggregate derivative query did not have a value in the first
time bucket, it would abort early and return a single row with value
of 0.  Similarly, if either the current or previous value was nil,
it would skip the row and not append any values causing gaps and
no data to show up.

Instead, this will append a nil value if either the current or previous
valis is nil.  This essentially allows nil values to carry through the
results as well as gives a more sensible value for rows where we cannot
compute a difference (instead of dropping the row as before).

Fixes #4237 #4263
@@ -1044,22 +1053,6 @@ func ProcessAggregateDerivative(results [][]interface{}, isNonNegative bool, int
}
}

// Check the value's type to ensure it's an numeric, if not, return a 0 result. We only check the first value
Copy link
Contributor

Choose a reason for hiding this comment

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

So this was wrong, it simply should have been done in the loop for every value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that is part of it. The current code has the side effect of returning early if the first values nil too.

@otoolep
Copy link
Contributor

otoolep commented Oct 1, 2015

1 question, but generally looks sensible to me. +1

jwilder added a commit that referenced this pull request Oct 1, 2015
Handle missing values in aggregate derivative queries better
@jwilder jwilder merged commit 54ff199 into master Oct 1, 2015
@jwilder jwilder deleted the jw-derivative branch October 1, 2015 19:45
@jwilder
Copy link
Contributor Author

jwilder commented Oct 1, 2015

Thanks @otoolep and @caldwell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants