refactor: move CteWorkTable, default_table_source a bunch of files out of core
#15316
Conversation
CteWorkTable, default_table_source a bunch of files out of core
alamb
left a comment
There was a problem hiding this comment.
Thank you very much @logan-keede -- this looks good to me too
| use std::any::Any; | ||
| use std::sync::Arc; | ||
|
|
||
| /// Simple in-memory implementation of a schema. |
There was a problem hiding this comment.
this is nice to break this out into a new module
| } | ||
|
|
||
| /// Create a physical sort expression from a logical expression | ||
| pub fn create_physical_sort_expr( |
There was a problem hiding this comment.
this makes sense to move into physical expr too
|
where does Memtable belong datasource or catalog? it is TableProvider implementation so I thought It was going to be in catalog, but I m not so sure anymore as it has dependency on datasource. |
I think MemTable beloings in datasource (as it is a table provider) Since it is so simple / has very few dependencies I think it could also be in catalog so we have a simple implementation of an in memory system without needing stuff in datasource. However, I am not sure it really matters |
|
I thought it mattered because |
I don't know of any
So are you happy with this PR as is now? Does it make more sense? |
Yes, but I think we should consider moving In-memory format's providers from |
Which issue does this PR close?
datafusioncrate (datafusion/core) #14444.Rationale for this change
cte_worktable is a TableProvider.
default_table_source is a struct used to adapt TableProvider(physical trait) to logical trait.
Both seems to fit well in
catalogcrate.What changes are included in this PR?
move
cte_worktableanddefault_table_source(and perhaps a few more) out of core and a few more things tomove
core/datasource/memory.rs(MemTable) todatasourcecrateAre these changes tested?
testing by Github CI
Are there any user-facing changes?
Nope.