Replacing SessionState with Session and progress towards moving FileFormatFactory out of datasource

[logan-keede]

Which issue does this PR close?

• part of move datafusion-catalog-listing out of core/src/datasource #14462
• Part of [Epic] Split datasources out from datafusion crate (datafusion/core) #14444.

Rationale for this change

1. SessionState needs to be replaced with Session in datasource to split it out of core.
2. get_file_format_factory needs to be implemented for Session

   datafusion/datafusion/core/src/datasource/file_format/mod.rs

   Line 64 in 304488d

   pub trait FileFormatFactory: Sync + Send + GetExt + Debug {

3. for 2, we need to move FileScanConfig to datafusion-catalog-listing(or some other place, but out of core)

What changes are included in this PR?

Refactoring,

• minor SessionState -> dyn Session + some downcast to be removed later.
• major move FileScanConfig to datafusion-catalog-listing

Are these changes tested?

Are there any user-facing changes?

There should not be

[logan-keede]

maybe, keeping a separate file for addition on old file_scan_config would be better?

[logan-keede]

@alamb do you think this is correct approach to make progress?
PS: I am working on fixing CI

[davisp]

I just a first pass super quick skim of this and I'm not seeing anything crazy. I'll have to read through again while I have time to think harder and double check the places where code is moving before I give a +1 though,.

[alamb]

this is great

[alamb]

this is unfortunate -- is there any way we can avoid the (direct) avro dependency on the listing table? If not it is fine for this PR I was just wondering

[alamb]

Unfortunately, I think this OR had major conflicts with

• Introduce unified DataSourceExec for provided datasources, remove ParquetExec, CsvExec, etc #14224

@logan-keede is there any chance you are willing to take a look at the new structure and rework this PR?

[logan-keede]

Unfortunately, I think this OR had major conflicts with

• Introduce unified DataSourceExec for provided datasources, remove ParquetExec, CsvExec, etc #14224

@logan-keede is there any chance you are willing to take a look at the new structure and rework this PR?

Sure, I was kinda expecting this.

[logan-keede]

https://github.com/apache/datafusion/blame/bab0f54daa99830339c0000a19c1b4e3489278ad/datafusion/core/src/datasource/physical_plan/file_scan_config.rs#L605

this downcast introduced in #14224, probably needs to be moved/removed(for this issue), but I am not sure where/how, or can we make do without it(like before)?

@mertak-synnada do you have any Idea?

[logan-keede]

One solution, I can think of right now, is to have multiple SpecificFileScanConfig structs and make current struct GenericFileScanConfig a trait and then add specific impl, among other thing that should add some flexibility for future work or user specific implementation. (if it works like I think, I am new to rust's idea of polymorphism)

[mertak-synnada]

this downcast introduced in #14224, probably needs to be moved/removed(for this issue), but I am not sure where/how, or can we make do without it(like before)?

Sorry I'm pretty new to this issue's context, Is it a problem because of dependency? If so, with an ugly solution, we can check the self.source.file_type() == "avro" here. Or we may create, another level of abstraction and ask that API the file type. In the early implementation of #14224, file_type() was returning a dyn FileType trait but since it was only being used in to_str and these kind type checkings we removed it.

Edit: On a second thought, calling self.source.supports_repartition() here and defining in FileSource

    fn supports_repartition(&self) -> bool {
        true
    }

And overriding this in AvroSource is a better solution

[logan-keede]

I figured it would be better to start anew, so #14543