use FileFormat::get_ext as the default file extension filter#12417
use FileFormat::get_ext as the default file extension filter#12417alamb merged 2 commits intoapache:mainfrom
Conversation
alamb
left a comment
There was a problem hiding this comment.
Thank you @waruto210 -- this makes sense to me and I think is a nice improvement.
I have a small API improvement suggestion, but I think we can do it as a follow on PR if you prefer.
Thanks again!
|
|
||
| let opt = ListingOptions::new(Arc::new(format)) | ||
| .with_file_extension("") | ||
| let mut opt = ListingOptions::new(Arc::new(format)) |
There was a problem hiding this comment.
Maybe we could make this a nicer API by adding with_file_extension_opt or something so this code could be like:
let opt = ListingOptions::new(Arc::new(format))
.with_file_extension_opt(file_ext)
.with_target_partitions(target_partitions);There was a problem hiding this comment.
Maybe we could make this a nicer API by adding
with_file_extension_optor something so this code could be like:let opt = ListingOptions::new(Arc::new(format)) .with_file_extension_opt(file_ext) .with_target_partitions(target_partitions);
I think your advice is great, I'm on vacation right now and I'll revise the code when I get back.
There was a problem hiding this comment.
Here is a PR with the proposed API: #12461
I have modified the code according to this API.
|
BTW I am holding off on merging this PR until we cut the 42 release candidate to avoid introducing last minute behavior changes |
022c8f6 to
ad55fdb
Compare
alamb
left a comment
There was a problem hiding this comment.
Thanks @waruto210 -- this is great. 🚀
Which issue does this PR close?
Closes #12378.
Rationale for this change
Using
FileFormat::get_extas the default file extension filter better meets user expectations.What changes are included in this PR?
Use
FileFormat::get_extas the default file extension filterAre these changes tested?
Yes
Are there any user-facing changes?
User do not need to specify the file extension if they choose the default value.