scovich commented on code in PR #8540:
URL: https://github.com/apache/arrow-rs/pull/8540#discussion_r2399890813
##########
parquet/src/arrow/schema/primitive.rs:
##########
@@ -132,35 +133,24 @@ fn from_parquet(parquet_type: &Type) -> Result<DataType> {
}
fn decimal_type(scale: i32, precision: i32) -> Result<DataType> {
- if precision <= DECIMAL128_MAX_PRECISION as i32 {
- decimal_128_type(scale, precision)
- } else {
- decimal_256_type(scale, precision)
- }
-}
-
-fn decimal_128_type(scale: i32, precision: i32) -> Result<DataType> {
+ // Convert and validate inputs once
let scale = scale
.try_into()
.map_err(|_| arrow_err!("scale cannot be negative: {}", scale))?;
-
let precision = precision
.try_into()
.map_err(|_| arrow_err!("precision cannot be negative: {}",
precision))?;
- Ok(DataType::Decimal128(precision, scale))
-}
-
-fn decimal_256_type(scale: i32, precision: i32) -> Result<DataType> {
- let scale = scale
- .try_into()
- .map_err(|_| arrow_err!("scale cannot be negative: {}", scale))?;
-
- let precision = precision
- .try_into()
- .map_err(|_| arrow_err!("precision cannot be negative: {}",
precision))?;
-
- Ok(DataType::Decimal256(precision, scale))
+ // Dispatch based on precision thresholds using DecimalType trait constants
+ if precision <= Decimal32Type::MAX_PRECISION {
Review Comment:
Downside is... the cast.
So we should still try to figure out a way to make the parquet reader pull
the correct type:
* either globally (some variation of the fix I first attempted here)
* or perhaps something more targeted in the parquet reader, which notices
the variant type extension and massages the footer schema accordingly
--
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]
