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

Fix queries that return raw data without aggregates. #1704

Merged
merged 4 commits into from
Feb 24, 2015
Merged

Conversation

pauldix
Copy link
Member

@pauldix pauldix commented Feb 24, 2015

This is an ugly hack, but it'll have to do until I finish the query engine refactoring. Please to review @benbjohnson

@pauldix
Copy link
Member Author

pauldix commented Feb 24, 2015

bah, fixing test errors. Forgot to run the entire suite before commit.

s.CreateDatabase("foo")
s.CreateRetentionPolicy("foo", &influxdb.RetentionPolicy{Name: "raw", Duration: 1 * time.Hour})
s.SetDefaultRetentionPolicy("foo", "raw")
s.CreateUser("susy", "pass", false)
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, we don't need to create "susy". We should remove this from the other tests too.

@otoolep
Copy link
Contributor

otoolep commented Feb 24, 2015

Don't fully grok the rationale of some of these changes, but code looks reasonable.

if err != nil {
return nil, err
}
stmt := e.stmt
Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, because you're only supporting 1 field for raw queries, I presume this is why Substatement is not being called.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because for raw queries there's only a single map function. You can get 1, more, or all fields in a raw query.

@otoolep
Copy link
Contributor

otoolep commented Feb 24, 2015

Some feedback, none mandatory. +1

I also removed the engine test for the raw planner. It's getting tested elsewhere and it would have been too difficult to make it work. Besides, that's getting replaced soon anyway.
pauldix added a commit that referenced this pull request Feb 24, 2015
Fix queries that return raw data without aggregates.
@pauldix pauldix merged commit fdce7ff into master Feb 24, 2015
@pauldix pauldix deleted the fix-raw-ordering2 branch February 27, 2015 22:42
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.

None yet

2 participants