Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Made GroupFilter optional in parquet'sRecordReader and added method to set it. #386

Merged
merged 2 commits into from
Sep 7, 2021

Conversation

jorgecarleitao
Copy link
Owner

@jorgecarleitao jorgecarleitao commented Sep 7, 2021

This is more expressive since filtering is optional.

Backward incompatible changes

The function RecordReader::try_new now expects Option<GroupFilter> instead of GroupFilter. If you were not using filtering, migrate to this by passing None. If you were filtering, pass Some(...) instead.

Copy link
Collaborator

@houqp houqp left a comment

Choose a reason for hiding this comment

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

Could we also expose RecordReader.metadata so it can be used to build the row group filter?

@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #386 (3abb7f8) into main (877739b) will increase coverage by 0.11%.
The diff coverage is 36.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #386      +/-   ##
==========================================
+ Coverage   81.12%   81.24%   +0.11%     
==========================================
  Files         331      331              
  Lines       21957    22215     +258     
==========================================
+ Hits        17813    18048     +235     
- Misses       4144     4167      +23     
Impacted Files Coverage Δ
src/io/parquet/read/record_batch.rs 81.25% <12.50%> (-5.42%) ⬇️
tests/it/io/parquet/mod.rs 94.88% <100.00%> (-0.10%) ⬇️
tests/it/io/parquet/read.rs 98.46% <100.00%> (ø)
tests/it/array/primitive/mutable.rs 100.00% <0.00%> (ø)
src/bitmap/mutable.rs 90.14% <0.00%> (+0.93%) ⬆️
src/array/primitive/mutable.rs 88.69% <0.00%> (+1.92%) ⬆️
src/compute/like.rs 66.92% <0.00%> (+6.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 877739b...3abb7f8. Read the comment docs.

@jorgecarleitao
Copy link
Owner Author

Could we also expose RecordReader.metadata so it can be used to build the row group filter?

done 👍

Copy link
Collaborator

@houqp houqp left a comment

Choose a reason for hiding this comment

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

LGTM

@jorgecarleitao jorgecarleitao merged commit d7bd957 into main Sep 7, 2021
@jorgecarleitao jorgecarleitao deleted the filter_groups branch September 7, 2021 07:11
@jorgecarleitao jorgecarleitao changed the title Made GroupFilter optional in parquet'sRecordReader and added method to set it. Made GroupFilter optional in parquet'sRecordReader and added method to set it. Sep 7, 2021
@jorgecarleitao
Copy link
Owner Author

This has been released under v0.5.0, @houqp

@houqp
Copy link
Collaborator

houqp commented Sep 9, 2021

I am using git commit in my local arrow2 migration branch at the moment, but good to know we could release at a much faster cadence :D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants