mbrobbel commented on code in PR #7889: URL: https://github.com/apache/arrow-rs/pull/7889#discussion_r2200283854
########## arrow-avro/src/codec.rs: ########## @@ -61,9 +61,25 @@ impl AvroDataType { } /// Returns an arrow [`Field`] with the given name + #[cfg(feature = "canonical_extension_types")] pub fn field_with_name(&self, name: &str) -> Field { - let d = self.codec.data_type(); - Field::new(name, d, self.nullability.is_some()).with_metadata(self.metadata.clone()) + let nullable = self.nullability.is_some(); + let data_type = self.codec.data_type(); + let field = match self.codec { + Codec::Uuid => Field::new(name, data_type, nullable) + .with_extension_type(arrow_schema::extension::Uuid), + _ => Field::new(name, data_type, nullable), + }; + field.with_metadata(self.metadata.clone()) + } + + /// Returns an arrow [`Field`] with the given name + #[cfg(not(feature = "canonical_extension_types"))] Review Comment: Nit: instead of duplicating the method (and overlapping logic) you could also create a separate method and call that only if the feature is enabled: ```rust #[cfg(feature = "canonical_extension_types")] fn with_extension_type(codec: &Codec, field: Field) -> Field { match codec { Codec::Uuid => Field::new(name, data_type, nullable) .with_extension_type(arrow_schema::extension::Uuid), _ => field, } } ``` ```rust pub fn field_with_name(&self, name: &str) -> Field { let field = ...; #[cfg(feature = "canonical_extension_types")] with_extension_type(&self.codec, field) #[cfg(not(feature = "canonical_extension_types"))] field } ``` -- 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