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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
