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