Added support for IPC 2.0 #168
Conversation
Codecov Report
@@ Coverage Diff @@
## main #168 +/- ##
==========================================
+ Coverage 75.63% 75.73% +0.09%
==========================================
Files 212 213 +1
Lines 18509 18603 +94
==========================================
+ Hits 14000 14089 +89
- Misses 4509 4514 +5
Continue to review full report at Codecov.
|
| buffer.as_mut_ptr() as *mut u8, | ||
| length * std::mem::size_of::<T>(), | ||
| ); | ||
| compression::decompress_lz4(&slice[8..], out_slice)? |
There was a problem hiding this comment.
If a writer tries to compress a buffer, and then it finds that the compressed buffer ratio isn't good enough, it can still write uncompressed buffers. In that case, we have to check if the first 8 bytes = -1i64. It's some of the details that I struggled with
There was a problem hiding this comment.
I am following the c++ implementation here, but I can't find that case there :/
There was a problem hiding this comment.
I also can't find it now, but I'll look in more detail; as I recall a review comment being raised in the Java implementation for this detal.
There was a problem hiding this comment.
Wouldn't it be fine for now to use the same as the c++ implementation does, and improve that behavior over a future PR? None of this results in UB; the worse case is a panic.
There was a problem hiding this comment.
I agree, it definitely would :)
Closes #163
This adds support for IPC format 2.0, which includes compression. The implementation is not the fastest, because it currently requires an extra allocation per buffer, but the roundtrip over the golden files passes, so we can live with it for now. ^_^
Note that this adds a new feature
io_ipc_compression, to activate the corresponding dependencies. These overlap withio_parquet, but it is still good to be aware of.