move information_schema to datafusion-catalog #14364
Conversation
github-actions
bot
added
core
|
I am checking this one out |
There was a problem hiding this comment.
Thank you @logan-keede -- this is great 🙏
I also took the liberty of pushing some commits to remove a few unecessary explciit dependencies as I thought that would be better than trying to explain how to do it
If you are feeling like some more refactoring projects, any chance you are interested in working to split out the data sources (aka make datafusion-datasource-parquet, datafusion-datasource-csv, etc)?
This doesn't have a ticket yet (as we were still working to extract the catalog and physical optimizers, but now that those are done, I think we are ready to try to break out the listing table / data sources)
datafusion/substrait/Cargo.toml
Outdated
| @@ -46,6 +46,7 @@ url = { workspace = true } | |||
|
|
|||
| [dev-dependencies] | |||
| datafusion = { workspace = true, features = ["nested_expressions"] } | |||
| datafusion-catalog = { workspace = true } | |||
There was a problem hiding this comment.
I think since datafusion re-exports datafusion_catalog we should be able to avoid this dependency:
There was a problem hiding this comment.
I knew there was some chance of removing this dependency, but not sure on how to go about it. So, thanks.
| /// assert_eq!(ctes[0].to_string(), "my_cte"); | ||
| /// ``` | ||
| pub fn resolve_table_references( | ||
| statement: &datafusion_sql::parser::Statement, |
There was a problem hiding this comment.
I think since this function belongs in datafusion-sql as it is walking over the sql parse tree
perhaps we can put it in https://github.com/apache/datafusion/tree/main/datafusion/sql/src/resolve.rs or something and then remove the dependency of datafusion-catalog on datafusion-sql
I think we could do this in a follow on PR as well
There was a problem hiding this comment.
let's do that then since I might not be able to commit over the next few days due to some personal commitments.
Well, I guess I know what I might be doing when I get back. I will definitely look into it, please let me know if there have been some discussion(or any relevant resource) on it in the past if that's not too much trouble. |
Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?