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

LIMIT seems broken #298

Merged
merged 1 commit into from
Mar 5, 2014
Merged

LIMIT seems broken #298

merged 1 commit into from
Mar 5, 2014

Conversation

jvshahid
Copy link
Contributor

@jvshahid jvshahid commented Mar 5, 2014

vers. 0.5-rc3

SELECT * FROM server_disks_1 limit 1

This query returns 12 results instead of one in the admin interface, "LIMIT 2" gives me 24. Same happens on other series too.

Those are the returned timestamps:

1393577978000
1393459205000
1392854405000
1392249604000
1391644808000
1391040004000
1390435202000
1390406899000
1381968002000
1381363201000
1380758405000
1380153601000

Do I get one result per shard?

@pauldix
Copy link
Member

pauldix commented Mar 5, 2014

Ah yes, it might be giving you one result per shard. We'll have a look

@pauldix pauldix added this to the 0.5.0 milestone Mar 5, 2014
// if we have a query with limit, then create an engine, or we can
// make the passthrough limit aware
shouldAggregateLocally = false
processor = engine.NewQueryEngine(querySpec.SelectQuery(), responseChan)
Copy link
Member

Choose a reason for hiding this comment

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

I think this is going to break in unexpected ways. Like if you do a percentile query, the engine will then try to compute percentiles on the already computed percentile results.

I think it's better to have the PassthroughEngine support limits and use that.

pauldix added a commit that referenced this pull request Mar 5, 2014
@pauldix pauldix merged commit 58cdd6f into master Mar 5, 2014
@jvshahid jvshahid deleted the fix-298-limit-with-multiple-shards branch March 25, 2014 16:39
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