refactor: add ExecutionPlan::file_scan_config to avoid downcasting

[not-my-profile]

I want to feature-gate AvroExec in a followup commit, so that you cannot get bitten by AvroExec::execute returning an NotImplemented error if the "avro" feature isn't enabled. (Since idiomatic Rust code should work if it compiles.)

As a preparation for that this commit gets rid of a plan_any.downcast_ref::<AvroExec>() call that couldn't be easily put behind a cfg(feature = "avro") without complicating the control flow.

Are there any user-facing changes?

Yes this PR adds the file_scan_config method to the ExecutionPlan trait.

[Dandandan]

Seems like a nice improvement to me

[not-my-profile]

I just slightly improved the doc comment for the new trait method:

-   /// Returns the [`FileScanConfig`] in case this is a physical execution plan or `None` otherwise.
+   /// Returns the [`FileScanConfig`] in case this is a data source scanning execution plan or `None` otherwise.

[alamb]

that certainly looks nicer

[alamb]

🤔 it seems like this has now introduced another dependency between datafusion and the physical plans (I am trying to cut out the physical plans into their own crate in #1754 )

I will futz with this and see if I can get an API that achieves both

[not-my-profile]

Thanks for bringing this up here. I disagree that this PR introduced any new dependency ... it just made the existing dependency explicit. Since I think you should of course be able to implement your own data source scanning execution plan (which previously you could not because of the hard-coded downcasting in the physical plan).

So I think the solution here is to move the new file_scan_config method to a new DataSource trait (not so sure about the name) that execution plans have to implement in order to use them with the physical plan. What do you think? (take what I'm saying with a grain of salt, I'm not that familiar with datafusion).

[alamb]

So I think the solution here is to move the new file_scan_config method to a new DataSource trait (not so sure about the name) that execution plans have to implement in order to use them with the physical plan. What do you think? (take what I'm saying with a grain of salt, I'm not that familiar with datafusion).

I think that is a great idea, and I will create a PR shortly for discussion

[alamb]

Thanks - I filed #7357 to track the work / continue the conversation so it wasn't lost on this merged PR. Let's continue the conversation there