neilconway commented on code in PR #23960:
URL: https://github.com/apache/datafusion/pull/23960#discussion_r3758388012
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -923,7 +923,18 @@ impl SessionState {
&self,
ext: &str,
) -> Option<Arc<dyn FileFormatFactory>> {
- self.file_formats.get(&ext.to_lowercase()).cloned()
+ let ext = ext.to_lowercase();
+ self.file_formats
+ .get(&ext)
+ .or_else(|| {
+ if ext ==
DEFAULT_ARROW_STREAM_EXTENSION.trim_start_matches('.') {
+ self.file_formats
+ .get(DEFAULT_ARROW_EXTENSION.trim_start_matches('.'))
+ } else {
+ None
+ }
+ })
+ .cloned()
Review Comment:
It seems a bit regrettable to add special-case logic for Arrow IPC into a
generic code path. The current approach also doesn't handle `COPY TO` for Arrow
IPC streams, which presumably might be helpful.
What if we added `ArrowStreamFormatFactory` as a parallel to
`ArrowFormatFactory`? Each format would then have a single extension, and we'd
have support for both input and output in a way that fits into the existing API
cleanly. The two Arrow format implementations might duplicate code, but maybe
we can refactor their implementations to share code where it makes sense. wdyt?
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -923,7 +923,18 @@ impl SessionState {
&self,
ext: &str,
) -> Option<Arc<dyn FileFormatFactory>> {
- self.file_formats.get(&ext.to_lowercase()).cloned()
+ let ext = ext.to_lowercase();
+ self.file_formats
+ .get(&ext)
+ .or_else(|| {
+ if ext ==
DEFAULT_ARROW_STREAM_EXTENSION.trim_start_matches('.') {
+ self.file_formats
+ .get(DEFAULT_ARROW_EXTENSION.trim_start_matches('.'))
+ } else {
+ None
+ }
+ })
+ .cloned()
Review Comment:
It seems a bit regrettable to add special-case logic for Arrow IPC into a
generic code path. The current approach also doesn't handle `COPY TO` for Arrow
IPC streams, which presumably might be useful.
What if we added `ArrowStreamFormatFactory` as a parallel to
`ArrowFormatFactory`? Each format would then have a single extension, and we'd
have support for both input and output in a way that fits into the existing API
cleanly. The two Arrow format implementations might duplicate code, but maybe
we can refactor their implementations to share code where it makes sense. wdyt?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
