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