-
Notifications
You must be signed in to change notification settings - Fork 1.6k
PARQUET-1310: Column indexes: Filtering #509
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
465cca6
PARQUET-1310: New options for column index based filtering
gszadovszky c00b2c0
PARQUET-1310: Prepare for skipping pages at reading
gszadovszky 3c9fe3e
PARQUET-1310: Simple implementation of page filtering
gszadovszky c471c72
PARQUET-1310: Glue the whole stuff together; implement proper tests
gszadovszky c0ce0f3
PARQUET-1310: Modifications according to zi's comments
gszadovszky 7d5faea
PARQUET-1310: Implement binary search for ASCENDING/DESCENDING orders
gszadovszky f9b6ecc
PARQUET-1310: Modifications according to zi's comments
gszadovszky e82b9d5
PARQUET-1310: Update terminology of column index based filtering
gszadovszky 325e5f6
PARQUET-1310: Fix exception naming
gszadovszky 659c1a0
PARQUET-1310: Fix issue introduced at exception renaming
gszadovszky 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
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
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.
(nit) I would remove this convenience method as it is not only superfluous but also unnecessary on the "convenience" path, since true is already the default. With this method we have 3 ways of setting the value to true: not doing anything, calling useColumnIndexFilter(true) and calling useColumnIndexFilter().
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.
I've followed the pattern of the other options (e.g.
useRecordFilter(boolean)anduseRecordFilter()etc.). I think, it is better to be consistent.