azecevic000 commented on code in PR #10959:
URL: https://github.com/apache/arrow-rs/pull/10959#discussion_r4066093436
##########
arrow-ipc/src/compression.rs:
##########
@@ -375,14 +386,17 @@ fn decompress_zstd(
))
}
-/// Get the uncompressed length
-/// Notes:
-/// LENGTH_NO_COMPRESSED_DATA: indicate that the data that follows is not
compressed
-/// 0: indicate that there is no data
-/// positive number: indicate the uncompressed length for the following data
-/// Returns an error if the input buffer is shorter than 8 bytes
+/// Reads the uncompressed length from a compressed IPC buffer.
+///
+/// A positive value is the exact expected uncompressed length, `0` indicates
+/// that there is no data, and `-1` indicates that the bytes following the
+/// prefix are not compressed.
+/// IPC decompression limits its output buffer to a positive advertised length
+/// and rejects output whose length differs.
+///
+/// Returns an error if the input buffer is shorter than the 8-byte prefix.
#[inline]
-fn read_uncompressed_size(buffer: &[u8]) -> Result<i64, ArrowError> {
+pub fn read_uncompressed_size(buffer: &[u8]) -> Result<i64, ArrowError> {
Review Comment:
@Jefffrey Thanks for taking a look at this PR. It's not about the complexity
of the function that would make this more convenient for downstream users. By
staying private, any downstream user that would mimic this function would
depend on a lot of implementation details that might change in the future (its
first 8 bytes, its little-endian, etc.). So `pub` abstracts all of those
details and leaves you just with `uncompressed_size: i64`.
I explained my use-case in the comments above, and I dug deeper after those
initial comments, and tbh I also wouldn't make this `pub` today, because it's
still too low-level, it's just the uncompressed size of a single buffer, what I
would ideally want is an API that gives me an uncompressed size of the whole
`RecordBatch`.
I will remove this `pub` change from this PR, and let's merge the LZ4 fix
after that if you agree. I will open a feature request issue with a description
of proposed changes, and I can open a separate PR with proposed changes. I'm
very willing to work on this if we can agree on a general direction. I would
like to get your feedback on that.
@Rich-T-kid Thanks again for pointing
https://github.com/apache/arrow-rs/issues/10392 to me. When I returned to it a
few days after that comment, it actually made me think deeper about the problem
at hand, and it's definitely related in some lower-level sense, and even
clarified a few things for me.
--
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]
