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


##########
datafusion/core/src/execution/context.rs:
##########
@@ -816,7 +809,7 @@ impl SessionContext {
             name,
             table_path,
             listing_options,
-            options.schema,
+            options.schema.map(|s| Arc::new(s.to_owned())),

Review Comment:
   `register_avro` and `register_json` methods take `AvroReadOptions` and 
`NdJsonReadOptions` as arguments respectively. When we try to unify them with 
`CsvReadOptions`, this change is obliged since 
   
   ```
   pub struct CsvReadOptions<'a> {
     /// An optional schema representing the CSV files. If None, CSV reader 
will try to infer it
     /// based on data in file.
     pub schema: Option<&'a Schema>,
   }
   ```
   and we do not want to break the CSV API. If it is OK to break CSV API, your 
suggestions are incredibly logical.



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