alamb commented on code in PR #17643:
URL: https://github.com/apache/datafusion/pull/17643#discussion_r2360596413


##########
datafusion-examples/examples/expr_api.rs:
##########
@@ -518,10 +518,7 @@ fn type_coercion_demo() -> Result<()> {
         HashMap::new(),
     )?;
     let i8_array = Int8Array::from_iter_values(vec![0, 1, 2]);
-    let batch = RecordBatch::try_new(
-        Arc::new(df_schema.as_arrow().to_owned()),
-        vec![Arc::new(i8_array) as _],
-    )?;
+    let batch = RecordBatch::try_new((&df_schema).into(), 
vec![Arc::new(i8_array) as _])?;

Review Comment:
   I would personally find it easier to understand there were no `clone`s going 
on using this syntax (I don't think this is actually any different code):
   
   ```suggestion
       let batch = RecordBatch::try_new(Arc::clone(&df_schema.as_ref()), 
vec![Arc::new(i8_array) as _])?;
   ```



##########
datafusion/common/src/dfschema.rs:
##########
@@ -1070,16 +1070,27 @@ fn format_simple_data_type(data_type: &DataType) -> 
String {
 impl From<DFSchema> for Schema {
     /// Convert DFSchema into a Schema
     fn from(df_schema: DFSchema) -> Self {
-        let fields: Fields = df_schema.inner.fields.clone();
-        Schema::new_with_metadata(fields, df_schema.inner.metadata.clone())
+        (&df_schema).into()
     }
 }
 
 impl From<&DFSchema> for Schema {
     /// Convert DFSchema reference into a Schema
     fn from(df_schema: &DFSchema) -> Self {
-        let fields: Fields = df_schema.inner.fields.clone();
-        Schema::new_with_metadata(fields, df_schema.inner.metadata.clone())
+        df_schema.as_arrow().clone()
+    }
+}
+
+impl<'a> From<&'a DFSchema> for &'a Schema {

Review Comment:
   👍 
   



##########
datafusion-examples/examples/flight/flight_server.rs:
##########
@@ -98,7 +99,7 @@ impl FlightService for FlightServiceImpl {
                 let df = ctx.sql(sql).await.map_err(to_tonic_err)?;
 
                 // execute the query
-                let schema = df.schema().clone().into();
+                let schema: SchemaRef = df.schema().into();

Review Comment:
   Likewise here and below, I would find it easier 
   
   ```suggestion
                   let schema: SchemaRef = Arc::clone(df.schema.as_ref());
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to