-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Complete moving PhysicalOptimizer into datafusion-physical-optimizer
#14300
Conversation
There are some doc update work. I'll get ready this PR shortly |
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.
Thank you @berkaysynnada and @logan-keede for pushing this along -- epic work
I reviewed the last 4 commits in this PR (what is on top of #14298) and it looks great to me. I think this PR just needs a merge up from main and it will be good to go
Thank you also @logan-keede for the ticket links -- I have added them to the PR description
@@ -66,6 +66,7 @@ datafusion-expr = { workspace = true } | |||
datafusion-functions-window-common = { workspace = true } | |||
datafusion-optimizer = { workspace = true, default-features = true } | |||
datafusion-physical-expr = { workspace = true, default-features = true } | |||
datafusion-physical-optimizer = { workspace = true, default-features = true } |
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 just follows the existing patterns so I think it is ok, but I think in general the examples should only be using the DataFusion crate (to ensure all relevant structures are exposed)
I will make a follow on PR
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.
@@ -677,7 +677,6 @@ pub mod dataframe; | |||
pub mod datasource; | |||
pub mod error; | |||
pub mod execution; | |||
pub mod physical_optimizer; |
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.
🚀
datafusion-physical-optimizer
Done |
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.
EPIC!
@@ -66,6 +66,7 @@ datafusion-expr = { workspace = true } | |||
datafusion-functions-window-common = { workspace = true } | |||
datafusion-optimizer = { workspace = true, default-features = true } | |||
datafusion-physical-expr = { workspace = true, default-features = true } | |||
datafusion-physical-optimizer = { workspace = true, default-features = true } |
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.
Let's do this. Thanks again @berkaysynnada -- very nice |
Which issue does this PR close?
datafusion
core crate) #13814Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?