metesynnada commented on code in PR #4694:
URL: https://github.com/apache/arrow-datafusion/pull/4694#discussion_r1058116313


##########
datafusion/core/src/execution/options.rs:
##########
@@ -286,7 +314,7 @@ impl<'a> AvroReadOptions<'a> {
 #[derive(Clone)]
 pub struct NdJsonReadOptions<'a> {
     /// The data source schema.
-    pub schema: Option<SchemaRef>,
+    pub schema: Option<&'a Schema>,

Review Comment:
   I changed it while trying to standardize (unify) the CSV, JSON, and AVRO 
APIs. I didn't want to break the CSV API so I change others. You are completely 
right about the problem of the change. Would you suggest changing CSV's schema 
API (it will break many tests and probably user code.)?
   
   ```rust /// Specify schema to use for CSV read
       pub fn schema(mut self, schema: &'a Schema) -> Self {
           self.schema = Some(schema);
           self
       }
   ```



##########
datafusion/core/src/execution/options.rs:
##########
@@ -286,7 +314,7 @@ impl<'a> AvroReadOptions<'a> {
 #[derive(Clone)]
 pub struct NdJsonReadOptions<'a> {
     /// The data source schema.
-    pub schema: Option<SchemaRef>,
+    pub schema: Option<&'a Schema>,

Review Comment:
   I changed it while trying to standardize (and unify) the CSV, JSON, and AVRO 
APIs. I didn't want to break the CSV API so I change others. You are completely 
right about the problem of the change. Would you suggest changing CSV's schema 
API (it will break many tests and probably user code.)?
   
   ```rust /// Specify schema to use for CSV read
       pub fn schema(mut self, schema: &'a Schema) -> Self {
           self.schema = Some(schema);
           self
       }
   ```



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

Reply via email to