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

ARROW-6091: [Rust] [DataFusion] Implement physical execution plan for LIMIT #5509

Closed
wants to merge 5 commits into from

Conversation

andygrove
Copy link
Member

No description provided.

@andygrove andygrove requested a review from sunchao September 27, 2019 19:33
// collect up to "limit" rows on each partition
let threads: Vec<JoinHandle<Result<Vec<RecordBatch>>>> = self
.partitions
.iter()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be an overkill to use rayon's par_iter()?

Copy link
Member Author

Choose a reason for hiding this comment

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

For this round my plan was to keep things simple and do a second round of optimizations that I've started tracking in https://jira.apache.org/jira/browse/ARROW-6689 and in particular see the subtask https://jira.apache.org/jira/browse/ARROW-6691 related to threading.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's a "no" for rayon :D

@github-actions
Copy link

@andygrove
Copy link
Member Author

@nevi-me @sunchao @paddyhoran Please review when you can. Thanks.

@andygrove
Copy link
Member Author

@nevi-me @sunchao @paddyhoran Any objection to me merging this one?

Copy link
Contributor

@paddyhoran paddyhoran left a comment

Choose a reason for hiding this comment

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

I only did a high level review but LGTM. I'm in favor of merging and opening follow up issues if we find anything (particularly when it comes to your PR's to DataFusion) to keep the momentum going.

@andygrove andygrove closed this in d3ba809 Oct 3, 2019
kszucs pushed a commit that referenced this pull request Oct 5, 2019
… LIMIT

Closes #5509 from andygrove/ARROW-6091 and squashes the following commits:

cb5c622 <Andy Grove> bump nightly version
00078c7 <Andy Grove> minor optimization
0c91602 <Andy Grove> test passes
40e617f <Andy Grove> unit test
3cb4cab <Andy Grove> start roughing out LIMIT

Authored-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants