peterxcli opened a new issue, #11067:
URL: https://github.com/apache/arrow-rs/issues/11067

   ### Describe the bug
   
   The Parquet reader infers `Decimal256(38, 2)` for a shredded `typed_value` 
stored as a 17–32 byte `FIXED_LEN_BYTE_ARRAY`, even when the declared precision 
and every value fit Decimal128. `VariantArray::try_new` then rejects the 
reader's representation.
   
   Reproduced on arrow-rs `4cd8be954f6bc6b6dd265140207365b59a9900ec`, with no 
`ARROW:schema` metadata involved. The [Parquet shredding 
mapping](https://github.com/apache/parquet-format/blob/master/VariantShredding.md#shredded-value-types)
 permits FIXED_LEN_BYTE_ARRAY for decimal16; the physical width alone need not 
imply precision above 38.
   
   ### To Reproduce
   
   ```rust
   use std::sync::Arc;
   use arrow::array::{Array, ArrayRef, BinaryArray, Decimal256Array, 
StructArray};
   use arrow::datatypes::{DataType, Field, i256};
   use parquet::arrow::parquet_to_arrow_schema;
   use parquet::schema::{parser::parse_message_type, types::SchemaDescriptor};
   use parquet_variant_compute::VariantArray;
   
   fn main() {
       for width in [17, 32] {
           let schema = parse_message_type(&format!(
               "message test {{ required group v (VARIANT) {{
                   required binary metadata;
                   optional binary value;
                   optional fixed_len_byte_array({width}) typed_value 
(DECIMAL(38,2));
               }} }}"
           )).unwrap();
           let schema = 
parquet_to_arrow_schema(&SchemaDescriptor::new(Arc::new(schema)), 
None).unwrap();
           let DataType::Struct(fields) = schema.field(0).data_type() else { 
panic!() };
           assert_eq!(fields[2].data_type(), &DataType::Decimal256(38, 2));
   
           // Construct the inferred representation with values that fit 
Decimal128.
           for raw in [12345, -12345] {
               let columns: Vec<ArrayRef> = vec![
                   Arc::new(BinaryArray::from_vec(vec![&[1, 0, 0]])),
                   Arc::new(BinaryArray::from(vec![None::<&[u8]>])),
                   Arc::new(Decimal256Array::from(vec![i256::from_i128(raw)])
                       .with_precision_and_scale(38, 2).unwrap()),
               ];
               let fields = ["metadata", "value", "typed_value"].into_iter()
                   .zip(&columns).map(|(name, array)|
                       Arc::new(Field::new(name, array.data_type().clone(), 
name != "metadata")))
                   .collect::<Vec<_>>();
               let input = StructArray::new(fields.into(), columns, None);
               println!("{}", VariantArray::try_new(&input).unwrap_err());
           }
       }
   }
   ```
   
   Each case returns `Illegal shredded value type: Decimal256(38, 2)`.
   
   ### Expected behavior
   
   A supported reader-to-Variant path should reconstruct these decimals as 
decimal16. Possible fixes are checked Decimal256 narrowing in Variant input 
normalization for valid Variant precision/scale, or compatible Parquet 
inference/schema coercion. Preserve signed values, scale and nulls; report 
overflow or invalid precision rather than silently producing nulls. This does 
not request Variant support for precision above 38.
   
   ### Additional context
   
   Comet currently casts `Decimal256(p,s)` to `Decimal128(p,s)` for `p <= 38`. 
Its [file scan 
regression](https://github.com/apache/datafusion-comet/blob/6e556c944873554310aedd6a27f0d3e77ef17e71/native/core/src/parquet/parquet_exec/variant_tests.rs#L321-L355)
 writes positive and negative DECIMAL(38,2) values at both physical widths. 
Downstream removal tracker: apache/datafusion-comet#5477.
   
   #9984 / #9985 validate legal physical lengths, while #8549 concerns smaller 
decimal widths. Neither resolves this Variant ingestion failure; #10417 
concerns unsigned integers.
   


-- 
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]

Reply via email to