superserious-dev commented on code in PR #7779:
URL: https://github.com/apache/arrow-rs/pull/7779#discussion_r2167405648


##########
parquet-variant/src/decoder.rs:
##########
@@ -205,22 +206,40 @@ pub(crate) fn decode_int64(data: &[u8]) -> Result<i64, 
ArrowError> {
 /// Decodes a Decimal4 from the value section of a variant.
 pub(crate) fn decode_decimal4(data: &[u8]) -> Result<(i32, u8), ArrowError> {
     let scale = u8::from_le_bytes(array_from_slice(data, 0)?);

Review Comment:
   The scale being type `u8` ensures that negative values are not allowed. The 
upper bound is validated via `DECIMAL_MAX_SCALE`.



##########
parquet-variant/Cargo.toml:
##########
@@ -35,6 +35,7 @@ rust-version = "1.83"
 [dependencies]
 arrow-schema = { workspace = true }
 chrono = { workspace = true }
+paste = { version = "1.0" }

Review Comment:
   Used for writing the decoder test macros. This is already a dependency in 
the 
[parquet](https://github.com/apache/arrow-rs/blob/10d9714ff04eb3ef329e2e7a6f7edae16aa4f8ae/parquet/Cargo.toml#L69)
 crate, so bringing it into `parquet-variant`.



##########
parquet-variant/src/decoder.rs:
##########
@@ -25,6 +25,7 @@ use std::num::TryFromIntError;
 
 // Makes the code a bit more readable
 pub(crate) const VARIANT_VALUE_HEADER_BYTES: usize = 1;
+pub(crate) const DECIMAL_MAX_SCALE: u8 = 38;

Review Comment:
   According to the 
[spec](https://github.com/apache/parquet-format/blob/cf943c197f4fad826b14ba0c40eb0ffdab585285/VariantEncoding.md?plain=1#L418-L420),
 all the decimal variants have a max scale of 38.



##########
parquet-variant/src/decoder.rs:
##########
@@ -284,157 +303,253 @@ pub(crate) fn decode_short_string(metadata: u8, data: 
&[u8]) -> Result<ShortStri
 #[cfg(test)]
 mod tests {
     use super::*;
+    use paste::paste;
+
+    mod integer {

Review Comment:
   Created macros and grouped similar tests together in order to reduce some of 
the boilerplate test code.



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to