Implemented a ReadOptions trait for cleaner code. #5025
Conversation
…read_csv` and other read funcitons can now read using the same method.
There was a problem hiding this comment.
Thank you @saikrishna1-bidgely -- i agree this looks like a nice API change. I had a few small comments but otherwise I think it is ready go to
|
|
||
| #[async_trait] | ||
| /// |
There was a problem hiding this comment.
Can you please fill in this docstring?
There was a problem hiding this comment.
Updated the docs.
| /// Helper to convert these user facing options to `ListingTable` options | ||
| pub fn to_listing_options(&self, target_partitions: usize) -> ListingOptions { | ||
| fn to_listing_options(&self, target_partitions: usize) -> ListingOptions; |
There was a problem hiding this comment.
| fn to_listing_options(&self, target_partitions: usize) -> ListingOptions; | |
| fn to_listing_options(&self, config: &SessionConfig) -> ListingOptions; |
While I realize that the target_partitions is the only thing actually used at the moment, since this is part of the public API I think providing &SessionConfig would be more future proof (aka avoid changes to this trait over time)
There was a problem hiding this comment.
This looks really nice @saikrishna1-bidgely - thank you again ❤️
cc @tustvold / @andygrove / @thinkharderdev / @Dandandan
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
|
Thanks agian @saikrishna1-bidgely |
|
Benchmark runs are scheduled for baseline = bc9b78d and contender = 556fffb. 556fffb is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
The
read_csv,read_parquet,read_jsonandread_avromethods can not use a single method to read the locations.Which issue does this PR close?
Closes #5024.
Rationale for this change
This will make for a simpler code in #4908.
What changes are included in this PR?
Implemented a new trait
ReadOptions. The code for methodto_listing_optionshas been moved from the respective structs into this trait. Also the code fromread_csv/json/avro/parquetto get the resolved schema has been moved here inget_resolved_schema.Once this trait is implemented, the remaining code in the methods
read_csv/json/avro/parquetis the same and has been moved to a private method.Are these changes tested?
This is only a refactor and the existing tests should cover the refactored code.
Are there any user-facing changes?
No