alamb commented on code in PR #4411:
URL: https://github.com/apache/arrow-rs/pull/4411#discussion_r1232460412
##########
arrow-ipc/src/compression.rs:
##########
@@ -69,7 +69,7 @@ impl CompressionCodec {
output.extend_from_slice(&uncompressed_data_len.to_le_bytes());
self.compress(input, output)?;
- let compression_len = output.len();
+ let compression_len = output.len() - original_output_len;
Review Comment:
> The wrongly calculated compressed length included the full original buffer
length, which will decline almost all the compressable data. Suppose original
buffer len is a, incoming data len is b, compressed data len is c, the code
should compare b and c instead of b and a+c
I agree this makes much more sense. I carefully reviewed this code and it
looks good to me. THank you @lwpyr
It would be really nice if you could figure out some way to test it (so that
we don't break it inadvertently in the future), but given this feature has no
explicitly coverage (given the fact that there are no tests that need to
change) I think this OR is ok
cc @liukun4515
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
