Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Feb 24, 2021

Rationale

Goal is to consolidate the expression walking in one place as part of my longer term plan to make plan rewriting avoid so much copying in the DataFusion optimizer (https://issues.apache.org/jira/browse/ARROW-11689). This is setting the stage for LogicalPlans to be able to rewrite themselves rather than being rewritten by the optimizer utils

Changes

move expressions and inputs into LogicalPlan rather than helpers in util. There are no logic changes.

@github-actions
Copy link

@codecov-io
Copy link

Codecov Report

Merging #9568 (aefb5c2) into master (5bea624) will increase coverage by 0.12%.
The diff coverage is 89.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9568      +/-   ##
==========================================
+ Coverage   82.25%   82.38%   +0.12%     
==========================================
  Files         244      244              
  Lines       55685    56221     +536     
==========================================
+ Hits        45806    46319     +513     
- Misses       9879     9902      +23     
Impacted Files Coverage Δ
rust/datafusion/src/logical_plan/expr.rs 81.56% <ø> (+0.42%) ⬆️
...ion-testing/src/bin/arrow-json-integration-test.rs 0.00% <0.00%> (ø)
rust/arrow/src/ipc/writer.rs 87.23% <50.00%> (-0.59%) ⬇️
rust/datafusion/src/logical_plan/plan.rs 79.59% <58.53%> (-2.86%) ⬇️
...datafusion/src/physical_plan/string_expressions.rs 77.00% <82.23%> (+7.37%) ⬆️
rust/parquet/src/arrow/array_reader.rs 77.61% <91.30%> (-0.02%) ⬇️
rust/datafusion/src/physical_plan/functions.rs 85.52% <91.42%> (+11.69%) ⬆️
rust/datafusion/src/optimizer/constant_folding.rs 94.46% <100.00%> (ø)
rust/datafusion/src/optimizer/filter_push_down.rs 97.65% <100.00%> (ø)
...datafusion/src/optimizer/hash_build_probe_order.rs 55.31% <100.00%> (ø)
... and 20 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 4beb514...aefb5c2. Read the comment docs.

@alamb alamb requested a review from andygrove February 25, 2021 11:34
@alamb
Copy link
Contributor Author

alamb commented Feb 25, 2021

FYI @Dandandan and @houqp

Copy link
Contributor

@Dandandan Dandandan left a comment

Choose a reason for hiding this comment

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

Makes sense, LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants