-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-4605: [Rust] Move filter and limit code from DataFusion into compute module #3741
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
Closed
Closed
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
d216fa0
Move filter and limit to array_ops
0ca0412
Rewrite filter and limit using macros
32a2f85
Add tests for limit and filter
b20ea6d
Do bound checking in limit function
2a389a3
create BinaryArray directly from byte slice to prevent converting to …
nevi-me 6422e18
cargo fmt
nevi-me 2f44a8a
Use the size of the array as limit instead of returning an error
2e9616b
Add documentation for the limit function
5a1047c
Name variables consistently
58d1f5c
Merge branch 'ARROW-4605' into ARROW-4605
ntrinquier 728884b
Merge pull request #1 from nevi-me/ARROW-4605
ntrinquier f0578f6
Add tests for limit and filter with BinaryArray
257d235
Add support for null values in limit and filter
344379a
Initialize vectors with a capacity
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last nit, we could return the array as
ArrayRefimmediately if the limit >= len. I sold have thought of it earlier, my apologies.I'm happy with everything else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can help me here: how can I wrap the reference to
arrayin anArc<Array>/ArrayRef? Since the reference toarraycan be freed at any time it would leave theArcinvalid, so I have to specify the lifetime somehow. Can I do that withArc?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you're right, I missed that part. We can improve limit when we have zero-copy array slicing 👍🏾