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

Reply via email to