alamb commented on a change in pull request #1451:
URL: https://github.com/apache/arrow-rs/pull/1451#discussion_r837825084



##########
File path: arrow/src/json/reader.rs
##########
@@ -580,6 +581,8 @@ pub struct Decoder {
     projection: Option<Vec<String>>,
     /// Batch size (number of records to load each time)
     batch_size: usize,
+    /// optional HashMap of column names to its format strings

Review comment:
       ```suggestion
       /// optional HashMap of column names to its format string
   ```

##########
File path: arrow/src/json/reader.rs
##########
@@ -589,11 +592,13 @@ impl Decoder {
         schema: SchemaRef,
         batch_size: usize,
         projection: Option<Vec<String>>,
+        format_strings: Option<HashMap<String, String>>,

Review comment:
       I can't help but wonder if we would be better off allowing users to 
provide their own parsers to support the most general case. For example, what 
if this was
   
   ```suggestion
           formaters: Option<HashMap<String, Arc<dyn Parser>>,
   ```
   
   And then the user could pass whatever custom parser they wanted to the JSON 
reader?
   
   

##########
File path: arrow/src/json/reader.rs
##########
@@ -38,7 +38,7 @@
 //!
 //! let file = File::open("test/data/basic.json").unwrap();
 //!
-//! let mut json = json::Reader::new(BufReader::new(file), Arc::new(schema), 
1024, None);
+//! let mut json = json::Reader::new(BufReader::new(file), Arc::new(schema), 
1024, None, None);

Review comment:
       🤔  it is probably time to make some sort of `JsonReaderOptions` struct 
so we can add new options without changing the public API




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