[GitHub] [arrow-rs] alamb commented on a diff in pull request #2160: parquet reader: Support reading decimals from parquet `BYTE_ARRAY` type

[GitBox <gi...@apache.org>]

alamb commented on code in PR #2160:
URL: https://github.com/apache/arrow-rs/pull/2160#discussion_r929236773


##########
parquet/src/arrow/array_reader/builder.rs:
##########
@@ -233,6 +225,18 @@ fn build_primitive_reader(
                 column_desc,
                 arrow_type,
             ),
+            Some(DataType::Decimal(precision, scale)) => {
+                // read decimal data from parquet binary physical type
+                let convert = DecimalByteArrayConvert::new(DecimalArrayConverter::new(precision as i32, scale as i32));

Review Comment:
   Is this the core change?
   
    (namely to support reading arrow `DataType::Decimal` from parquet `BYTE_ARRAY` type where previously the code only supported `DataType::Decimal` in `FIXED_LEN_BYTE_ARRAY fields)?



##########
parquet/src/arrow/schema.rs:
##########
@@ -531,6 +531,32 @@ mod tests {
         assert_eq!(&arrow_fields, converted_arrow_schema.fields());
     }
 
+    #[test]
+    fn test_decimal_fields() {
+        let message_type = "
+        message test_schema {
+                    REQUIRED INT32 decimal1 (DECIMAL(4,2));
+                    REQUIRED INT64 decimal2 (DECIMAL(12,2));
+                    REQUIRED FIXED_LEN_BYTE_ARRAY (16) decimal3 (DECIMAL(30,2));
+                    REQUIRED BYTE_ARRAY decimal4 (DECIMAL(33,2));
+        }
+        ";
+
+        let parquet_group_type = parse_message_type(message_type).unwrap();
+
+        let parquet_schema = SchemaDescriptor::new(Arc::new(parquet_group_type));
+        let converted_arrow_schema =
+            parquet_to_arrow_schema(&parquet_schema, None).unwrap();
+
+        let arrow_fields = vec![
+            Field::new("decimal1", DataType::Decimal(4,2), false),
+            Field::new("decimal2", DataType::Decimal(12,2), false),
+            Field::new("decimal3", DataType::Decimal(30,2), false),
+            Field::new("decimal4", DataType::Decimal(33,2), false),

Review Comment:
   this would this have previously failed?



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org