fix: fetch remainder of metadata if it is large #873
Conversation
wjones127
force-pushed
the
wjones127/856-large-manifest
branch
from
7afdc79 to
dd6bb46
Compare
This comment was marked as resolved.
This comment was marked as resolved.
wjones127
changed the title
wjones127
force-pushed
the
wjones127/856-large-manifest
branch
from
54442a2 to
2a94717
Compare
| }; | ||
|
|
||
| // Need to trim the magic number at end, and the non-manifest data at the beginning | ||
| let buf = buf.slice(4..buf.len() - 16); |
There was a problem hiding this comment.
What does this 4 stand for?
There was a problem hiding this comment.
hmm well my comment ", and the non-manifest data at the beginning" isn't right / specific. I'm not sure yet; all I knew is that it was skipped previously. I guess it's the u32 message length here:
Perhaps we should be reading that length and validating it?
There was a problem hiding this comment.
Added validation for that and a better comment 👍
| @@ -66,7 +66,7 @@ impl From<&pb::DataFile> for DataFile { | |||
| /// | |||
| /// A fragment is a set of files which represent the different columns of the same rows. | |||
| /// If column exists in the schema, but the related file does not exist, treat this column as `nulls`. | |||
| #[derive(Debug, Clone)] | |||
| #[derive(Debug, Clone, PartialEq)] | |||
There was a problem hiding this comment.
Why does it require PartialEq, do you need to sort / storing them in some containers?
There was a problem hiding this comment.
This was for the sake of testing (assert_eq requires this).
|
can we bump a new version to crates.io? |
|
@haoxins we will release a new version today. |
Fixes #856