alamb commented on code in PR #9985:
URL: https://github.com/apache/arrow-rs/pull/9985#discussion_r3283446161
##########
parquet/src/arrow/schema/primitive.rs:
##########
@@ -168,6 +168,16 @@ fn decimal_256_type(scale: i32, precision: i32) ->
Result<DataType> {
Ok(DataType::Decimal256(precision, scale))
}
+fn check_decimal_length(type_length: i32) -> Result<()> {
+ // Arrow's widest decimal is Decimal256, which needs at most 32 bytes.
+ if !(1..=32).contains(&type_length) {
Review Comment:
I found this a confusing way to check bounds -- I would expect something like
```rust
if type_length < 1 || type_length > 32 {
```
##########
parquet/src/arrow/schema/primitive.rs:
##########
@@ -313,23 +323,29 @@ fn from_fixed_len_byte_array(
precision: i32,
type_length: i32,
) -> Result<DataType> {
- // TODO: This should check the type length for the decimal and interval
types
match (info.logical_type_ref(), info.converted_type()) {
(Some(LogicalType::Decimal { scale, precision }), _) => {
+ check_decimal_length(type_length)?;
Review Comment:
should this be checking exact lengths? The only valid lengths in this case
are 16 and 32 , right?
--
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]
