-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add downcast_to_source
method for DataSourceExec
#15416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I added the change to the upgrading doc to let users find it easily: 4682fcd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @xudong963 -- I agree this is a nice improvement. I don't think we should mention this function in the 46 upgrade guide given that the function isn't available until 47
datafusion/datasource/src/source.rs
Outdated
@@ -230,4 +231,18 @@ impl DataSourceExec { | |||
Boundedness::Bounded, | |||
) | |||
} | |||
|
|||
/// Downcast the `DataSourceExec` to a specific file source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Downcast the `DataSourceExec` to a specific file source | |
/// Downcast the `DataSourceExec`'s `data_source` to a specific file source | |
/// | |
/// Returns `None` if | |
/// 1. the datasource is not scanning files (`FileScanConfig`) | |
/// 2. The [`FileScanConfig::file_source`] is not of type <T> |
@@ -129,6 +129,20 @@ if let Some(datasource_exec) = plan.as_any().downcast_ref::<DataSourceExec>() { | |||
# */ | |||
``` | |||
|
|||
There's also a more convenient helper method `downcast_to_file_source` on `DataSourceExec` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code will not be available until DataFusion 47, so we probably need to put this into a new heading for 47 upgrade guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, i totally forgot that 🤦♂️
@mertak-synnada @alamb Thanks for your review! |
* Add downcast_to_source method for DataSourceExec * rename * fix conflicts * fix cippy * add the change to upgrading doc * prettier * remove * address comments
* Add downcast_to_source method for DataSourceExec * rename * fix conflicts * fix cippy * add the change to upgrading doc * prettier * remove * address comments
Rationale for this change
When I upgraded the df46, It was annoying for me to do the following thing(a series of
downcast_ref
) and it's also easy to call the wrong method, such as mixingfile_source()
anddata_source()
:What changes are included in this PR?
Add the
downcast_to_source
method forDataSourceExec
to make life easyAre these changes tested?
Yes, I replace the existing code with the new method.
Are there any user-facing changes?
It'll be useful for users to upgrade df46.