Skip to content
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

Move physical expression planning to datafusion-physical-expr crate #2682

Merged
merged 5 commits into from
Jun 4, 2022

Conversation

andygrove
Copy link
Member

@andygrove andygrove commented Jun 2, 2022

Which issue does this PR close?

Closes #2535

Rationale for this change

The main motivation was to not have SimplifyExpressions depend on the core datafusion crate so that we can move it to the new datafusion-optimizer crate

What changes are included in this PR?

Move the following items from core to datafusion-physical-expr:

  • create_physical_expr
  • ExecutionProps
  • VarProvider
  • ScalarFunctionExpr

Are there any user-facing changes?

Not too much since I added re-exports but this is still an API change.

@andygrove andygrove added the api change Changes the API exposed to users of the crate label Jun 2, 2022
@andygrove andygrove self-assigned this Jun 2, 2022
@github-actions github-actions bot added core Core DataFusion crate datafusion Changes in the datafusion crate optimizer Optimizer rules physical-expr Physical Expressions labels Jun 2, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #2682 (f3d5d71) into master (49e072a) will increase coverage by 0.00%.
The diff coverage is 86.22%.

@@           Coverage Diff           @@
##           master    #2682   +/-   ##
=======================================
  Coverage   84.63%   84.64%           
=======================================
  Files         267      269    +2     
  Lines       46867    46926   +59     
=======================================
+ Hits        39665    39719   +54     
- Misses       7202     7207    +5     
Impacted Files Coverage Δ
datafusion/core/src/execution/context.rs 78.78% <ø> (-0.10%) ⬇️
...afusion/core/src/optimizer/simplify_expressions.rs 82.00% <ø> (ø)
datafusion/core/src/physical_optimizer/pruning.rs 93.77% <ø> (ø)
datafusion/core/src/physical_plan/mod.rs 88.00% <ø> (ø)
datafusion/core/src/physical_plan/planner.rs 77.51% <ø> (-1.82%) ⬇️
datafusion/physical-expr/src/from_slice.rs 100.00% <ø> (ø)
datafusion/physical-expr/src/functions.rs 94.19% <ø> (+28.57%) ⬆️
datafusion/physical-expr/src/type_coercion.rs 98.73% <ø> (ø)
datafusion/physical-expr/src/udf.rs 100.00% <ø> (ø)
datafusion/physical-expr/src/scalar_function.rs 65.62% <65.62%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49e072a...f3d5d71. Read the comment docs.

@andygrove andygrove removed the datafusion Changes in the datafusion crate label Jun 3, 2022
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I obviously didn't review this entire PR in detail but I checked that the tests remained intact and everything else looks good. Thank you @andygrove I really like how the modularization is proceeding

@alamb alamb merged commit 1152c14 into apache:master Jun 4, 2022
@andygrove andygrove deleted the refactor-create-phys-expr branch January 27, 2023 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api change Changes the API exposed to users of the crate core Core DataFusion crate optimizer Optimizer rules physical-expr Physical Expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logical optimizer rule "simplify expressions" should not depend on the core datafusion crate
3 participants