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

Support sorting by time desc #3986

Merged
merged 13 commits into from
Sep 4, 2015
Merged

Support sorting by time desc #3986

merged 13 commits into from
Sep 4, 2015

Conversation

jwilder
Copy link
Contributor

@jwilder jwilder commented Sep 4, 2015

This PR adds support for sorting results by time in descending order. Two syntaxes are allowed:

select value from cpu order by time desc
select value from cpu order by desc

This allows queries to find the most recent value instead of having to query the whole data set to see the last value. Currently only sorting by time is allowed on raw and aggregate queries. If there are multiple sort fields, a parse error will be returned. Similarly, if a sort field other than time is specified, a parse error is returned.

Fixes #2022

@otoolep
Copy link
Contributor

otoolep commented Sep 4, 2015

Cool.

Are all the true and false required in the tests? Are there not enums for those?

@jwilder
Copy link
Contributor Author

jwilder commented Sep 4, 2015

Looks like I forgot to switch those over to the enum. Will fix.

@otoolep
Copy link
Contributor

otoolep commented Sep 4, 2015

Should we expect a query performance hit for normal time-ascending queries? You should probably check it, if you haven't already.


// order by time desc
&Query{
name: "order by time desc",
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe aggregate order by time desc?

@corylanou
Copy link
Contributor

Very nice. Great test coverage. +1

@jwilder
Copy link
Contributor Author

jwilder commented Sep 4, 2015

@otoolep I'll check. Ascending should perform as before for both raw and aggregate queries. Desc raw w/ bz1 has a little bit of overhead to create an index into the compressed buffer but should not add much. Desc aggregate queries should perform identical to ascending.

@jwilder
Copy link
Contributor Author

jwilder commented Sep 4, 2015

@otoolep Performance for raw and aggregate queries for asc and desc is the same from master to my branch using 10M points. Desc doesn't seem to have much overhead either which is good.

jwilder added a commit that referenced this pull request Sep 4, 2015
@jwilder jwilder merged commit 6f41c0f into master Sep 4, 2015
@jwilder jwilder deleted the jw-order-by branch September 4, 2015 15:43
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