alamb commented on code in PR #7779:
URL: https://github.com/apache/arrow-rs/pull/7779#discussion_r2167691309
##########
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:
If it is only used in tests, perhaps we can move it to `dev-dependencies`
```yaml
[dev-dependencies]
paste = { version = "1.0" }
```
##########
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 {
+ use super::*;
+
+ macro_rules! decoder_tests {
+ ($test_name:ident, $data:expr, $decode_fn:ident, $expected:expr)
=> {
+ paste! {
+ #[test]
+ fn [<$test_name _exact_length>]() {
+ let result = $decode_fn(&$data).unwrap();
+ assert_eq!(result, $expected);
+ }
+
+ #[test]
+ fn [<$test_name _truncated_length>]() {
+ // Remove the last byte of data so that there is not
enough to decode
+ let truncated_data = &$data[.. $data.len() - 1];
+ let result = $decode_fn(&truncated_data);
+ assert!(matches!(result,
Err(ArrowError::InvalidArgumentError(_))));
Review Comment:
I suggest an assert like
```rust
assert!(result.unwrap_err().to_string(), "Expected message");
```
Which will make it easier to assert the error is as expected
##########
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:
- I think this somewhat overlaps with @scovich 's PR here:
https://github.com/apache/arrow-rs/pull/7779
https://github.com/apache/arrow-rs/pull/7779 does not include any decoder
tests though
I think we could potentially avoid checking the scale/precision for decimals
in the decoder and let the higher level validation check them
--
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
