-
Notifications
You must be signed in to change notification settings - Fork 750
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
Do not assume dictionaries exists in footer #1631
Conversation
arrow/src/ipc/reader.rs
Outdated
"Expecting DictionaryBatch in dictionary blocks, found {:?}.", | ||
t | ||
))); | ||
match footer.dictionaries() { |
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.
dictionaries
is not a required field in the footer. This looks good fix.
There are some format error. |
Codecov Report
@@ Coverage Diff @@
## master #1631 +/- ##
=======================================
Coverage 83.02% 83.03%
=======================================
Files 193 193
Lines 55577 55580 +3
=======================================
+ Hits 46145 46149 +4
+ Misses 9432 9431 -1
Continue to review full report at Codecov.
|
You can run |
thank you, should be good now! |
Thank you @pcjentsch |
❤️ |
This is my first Rust PR, please let me know if I could do this in a more idiomatic way!
Which issue does this PR close?
Closes #1335, on the
arrow-rs
side at least, Julia's Arrow.jl also writes mismatched versions (apache/arrow-julia#320).What changes are included in this PR?
IPC reader no longer assumes footer exists.
Are there any user-facing changes?
No.