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

WHERE fields must be decoded during aggregates #4789

Merged
merged 1 commit into from
Nov 14, 2015
Merged

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Nov 13, 2015

This change ensures that if there are any fields in the WHERE clause of an aggregate that are different from the fields in the SELECT clause, that the cursors also decode those fields. Otherwise WHERE clauses of the form SELECT f(w) FROM x WHERE y=z will return incorrect results

@otoolep
Copy link
Contributor Author

otoolep commented Nov 13, 2015

Real fix for #4701

@otoolep
Copy link
Contributor Author

otoolep commented Nov 13, 2015

@otoolep
Copy link
Contributor Author

otoolep commented Nov 13, 2015

Will cherry-pick this to 0.9.5 when merged to master.

This change ensures that if there are any fields in the WHERE clause of
an aggregate that are different from the fields in the SELECT clause,
that the cursors also decode those fields. Otherwise WHERE clauses of
the form 'SELECT f(w) FROM x WHERE y=z' will return incorrect results

Fixes issue #4701.
@@ -649,7 +649,7 @@ func (m *AggregateMapper) openMeasurement(mm *Measurement) error {
}

for i, key := range t.SeriesKeys {
fields := slices.Union(selectFields, m.fieldNames, false)
fields := slices.Union(slices.Union(selectFields, m.fieldNames, false), m.whereFields, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This looks a little ugly. Future refactoring perhaps?

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 actually think it looks good! What could be clearer? :-)

@corylanou
Copy link
Contributor

Fix makes sense. +1

@benbjohnson
Copy link
Contributor

👍

otoolep added a commit that referenced this pull request Nov 14, 2015
WHERE fields must be decoded during aggregates
@otoolep otoolep merged commit 1228431 into master Nov 14, 2015
@gunnaraasen gunnaraasen deleted the multi_field_agg branch April 20, 2018 16:35
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.

3 participants