jecsand838 commented on code in PR #8220:
URL: https://github.com/apache/arrow-rs/pull/8220#discussion_r2302208952


##########
arrow-avro/src/reader/record.rs:
##########
@@ -1471,4 +1753,196 @@ mod tests {
         assert!(int_array.is_null(0)); // row1 is null
         assert_eq!(int_array.value(1), 42); // row3 value is 42
     }
+
+    fn make_record_resolved_decoder(

Review Comment:
   @alamb I'm planning to include  `Maker::resolve_type` method branches for 
the complex and logical types over my next few PRs (for example the enum 
mapping PR I put up has support for the `Enum` branch). These branches just 
have additional logic and I didn't want to balloon this PR.
   
   That being said I probably should have included placeholders and then in 
turn tests able to reach the `Skipper::from_avro` method as a part of this PR. 
That's definitely my mistake. 
   
   So I went ahead and added those placeholder branches and created an Avro 
file that covers every type currently supported by `arrow-avro` using this 
python script: 
https://gist.github.com/jecsand838/82d9874a5f9be8a636dcd49ad9b8e237
   
   Then I added a new `test_skippable_types_project_each_field_individually` 
test to the `arrow-avro/src/reader/mod.rs` file. This test behaves as you 
recommended in your other comment. Once the `arrow-avro` `Writer` has full type 
support, we can move towards a round trip approach as well. However the changes 
I just pushed up should include coverages for each skipping each of those types 
now.
   
   Thank you for catching this and calling it out!
   



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