azecevic000 commented on code in PR #10959:
URL: https://github.com/apache/arrow-rs/pull/10959#discussion_r4070146727


##########
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:
   Reverted `pub` change, only LZ4 fix remains in this PR.
   
   > given its part of the IPC format specification, its more than a private 
implementation detail anyway
   
   Yeah, you're right, being part of the specification (with trivial 
implementation) makes this a non-issue, as even if a major version upgrade of 
the specification happens, it will surely have to provide backwards 
compatibility for quite some time.
   
   > what I would ideally want is an API that gives me an uncompressed size of 
the whole RecordBatch
   
   I guess the specification also makes this kind of computation stable and not 
reliant on implementation details (arguably this might change it 
https://github.com/apache/arrow-rs/issues/10392), though it's not trivial, so 
when I have some initial proposition (hopefully by the end of the week), I will 
share it, and we can discuss if you see the value in taking the ownership of 
this kind of API.



-- 
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]

Reply via email to