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


##########
arrow-avro/src/reader/mod.rs:
##########
@@ -308,4 +315,129 @@ mod test {
             assert_eq!(read_file(&file, 3), expected);
         }
     }
+
+    #[test]
+    fn test_decimal() {
+        let files = [
+            ("avro/fixed_length_decimal.avro", 25, 2),

Review Comment:
   ❤️ 



##########
arrow-avro/src/reader/mod.rs:
##########
@@ -308,4 +315,129 @@ mod test {
             assert_eq!(read_file(&file, 3), expected);
         }
     }
+
+    #[test]
+    fn test_decimal() {
+        let files = [
+            ("avro/fixed_length_decimal.avro", 25, 2),
+            ("avro/fixed_length_decimal_legacy.avro", 13, 2),
+            ("avro/int32_decimal.avro", 4, 2),
+            ("avro/int64_decimal.avro", 10, 2),
+        ];
+        let decimal_values: Vec<i128> = (1..=24).map(|n| n as i128 * 
100).collect();
+        for (file, precision, scale) in files {
+            let file_path = arrow_test_data(file);
+            let actual_batch = read_file(&file_path, 8);
+            let expected_array = 
Decimal128Array::from_iter_values(decimal_values.clone())
+                .with_precision_and_scale(precision, scale)
+                .unwrap();
+            let mut meta = HashMap::new();
+            meta.insert("precision".to_string(), precision.to_string());
+            meta.insert("scale".to_string(), scale.to_string());
+            let field_with_meta = Field::new("value", 
DataType::Decimal128(precision, scale), true)
+                .with_metadata(meta);
+            let expected_schema = Arc::new(Schema::new(vec![field_with_meta]));
+            let expected_batch =
+                RecordBatch::try_new(expected_schema.clone(), 
vec![Arc::new(expected_array)])
+                    .expect("Failed to build expected RecordBatch");
+            assert_eq!(
+                actual_batch, expected_batch,
+                "Decoded RecordBatch does not match the expected Decimal128 
data for file {file}"
+            );
+            let actual_batch_small = read_file(&file_path, 3);
+            assert_eq!(
+                actual_batch_small,
+                expected_batch,
+                "Decoded RecordBatch does not match the expected Decimal128 
data for file {file} with batch size 3"
+            );
+        }
+    }
+
+    #[test]
+    fn test_simple() {
+        let tests = [
+            ("avro/simple_enum.avro", 4, build_expected_enum(), 2),
+            ("avro/simple_fixed.avro", 2, build_expected_fixed(), 1),
+        ];
+
+        fn build_expected_enum() -> RecordBatch {

Review Comment:
   Rather than building up the expected values manually, what do you think 
about  reading the corresponding expected results instead?
   
   https://github.com/apache/arrow-testing/blob/master/data/avro/README.md 
suggests  that the corresponding files are here: 
https://github.com/apache/parquet-testing/tree/master/data
   
   Doing this would result in adding a parquet (dev) dependency on this crate 
which might not be ideal 🤔 



##########
arrow-avro/src/reader/mod.rs:
##########
@@ -308,4 +315,129 @@ mod test {
             assert_eq!(read_file(&file, 3), expected);
         }
     }
+
+    #[test]
+    fn test_decimal() {
+        let files = [
+            ("avro/fixed_length_decimal.avro", 25, 2),
+            ("avro/fixed_length_decimal_legacy.avro", 13, 2),
+            ("avro/int32_decimal.avro", 4, 2),
+            ("avro/int64_decimal.avro", 10, 2),
+        ];
+        let decimal_values: Vec<i128> = (1..=24).map(|n| n as i128 * 
100).collect();
+        for (file, precision, scale) in files {
+            let file_path = arrow_test_data(file);
+            let actual_batch = read_file(&file_path, 8);
+            let expected_array = 
Decimal128Array::from_iter_values(decimal_values.clone())
+                .with_precision_and_scale(precision, scale)
+                .unwrap();
+            let mut meta = HashMap::new();
+            meta.insert("precision".to_string(), precision.to_string());
+            meta.insert("scale".to_string(), scale.to_string());
+            let field_with_meta = Field::new("value", 
DataType::Decimal128(precision, scale), true)
+                .with_metadata(meta);
+            let expected_schema = Arc::new(Schema::new(vec![field_with_meta]));
+            let expected_batch =
+                RecordBatch::try_new(expected_schema.clone(), 
vec![Arc::new(expected_array)])
+                    .expect("Failed to build expected RecordBatch");
+            assert_eq!(
+                actual_batch, expected_batch,
+                "Decoded RecordBatch does not match the expected Decimal128 
data for file {file}"
+            );
+            let actual_batch_small = read_file(&file_path, 3);
+            assert_eq!(
+                actual_batch_small,
+                expected_batch,
+                "Decoded RecordBatch does not match the expected Decimal128 
data for file {file} with batch size 3"
+            );
+        }
+    }
+
+    #[test]
+    fn test_simple() {
+        let tests = [
+            ("avro/simple_enum.avro", 4, build_expected_enum(), 2),

Review Comment:
   this is great -- thank you for these tests



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

Reply via email to