Skip to content

ARROW-6058: [C++][Parquet] Validate whole ColumnChunk raw data reads so that underlying filesystem issues are caught earlier - #5137

Closed
wesm wants to merge 3 commits into
apache:masterfrom
wesm:ARROW-6058
Closed

ARROW-6058: [C++][Parquet] Validate whole ColumnChunk raw data reads so that underlying filesystem issues are caught earlier#5137
wesm wants to merge 3 commits into
apache:masterfrom
wesm:ARROW-6058

Conversation

@wesm

@wesm wesm commented Aug 20, 2019

Copy link
Copy Markdown
Member

Incomplete reads from files were not being caught here, which would result in an unhelpful exception being raised later when data pages could not be decoded. If the entire ColumnChunk read is incomplete here, then the chunk cannot be deserialized, so raising an error here is the proper thing to do. This also helped me debug the root cause of ARROW-6058

@wesm

wesm commented Aug 20, 2019

Copy link
Copy Markdown
Member Author

I'm going to add a unit test, pls hold off on merging

@wesm

wesm commented Aug 20, 2019

Copy link
Copy Markdown
Member Author

done

@codecov-io

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@a40872e). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5137   +/-   ##
=========================================
  Coverage          ?   75.26%           
=========================================
  Files             ?       57           
  Lines             ?     3619           
  Branches          ?        0           
=========================================
  Hits              ?     2724           
  Misses            ?      895           
  Partials          ?        0

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 a40872e...fe678c7. Read the comment docs.

@wesm

wesm commented Aug 21, 2019

Copy link
Copy Markdown
Member Author

+1

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