perf: fine–tuned plan steps #4258
Conversation
Signed-off-by: Ruihang Xia <[email protected]>
WalkthroughThe recent changes encompass updating the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DistExtensionPlanner
participant SessionState
participant LogicalPlan
participant PhysicalPlan
Client->>DistExtensionPlanner: Call optimize_input_logical_plan
DistExtensionPlanner->>SessionState: Use session_state
DistExtensionPlanner->>LogicalPlan: Optimize input logical plan
LogicalPlan-->>DistExtensionPlanner: Return optimized plan
Client->>DistExtensionPlanner: Call create_physical_plan
DistExtensionPlanner->>OptimizedPlan: Use optimized plan
OptimizedPlan-->>PhysicalPlan: Create physical plan
PhysicalPlan-->>DistExtensionPlanner: Return physical plan
DistExtensionPlanner-->>Client: Return result
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
Cargo.toml (1)
107-115: Issues found with the revision hashes inCargo.tomlThe specified revision hash
d7bda5c9b762426e81f144296deadc87e5f4a0b8for thedatafusiondependencies does not match any commit in thewaynexia/arrow-datafusionrepository. Please verify and update the revision hashes to ensure they are correct.
- Files/Lines impacted:
Cargo.toml, lines 107-115Analysis chain
LGTM! Updated repository URLs for
datafusiondependencies.The changes to update the repository URLs of several
datafusiondependencies from the Apache repository to thewaynexia/arrow-datafusionrepository are approved.However, ensure that the updated revision hashes are correct.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the revision hashes of the updated `datafusion` dependencies. # Test: Fetch the latest commit hashes from the `waynexia/arrow-datafusion` repository. # Expect: The fetched hashes should match the updated revision hashes in `Cargo.toml`. repos=( "datafusion" "datafusion-common" "datafusion-expr" "datafusion-functions" "datafusion-optimizer" "datafusion-physical-expr" "datafusion-physical-plan" "datafusion-sql" "datafusion-substrait" ) for repo in "${repos[@]}"; do git ls-remote https://github.com/waynexia/arrow-datafusion.git HEAD doneLength of output: 1496
Script:
#!/bin/bash # Description: Verify the specific revision hashes of the updated `datafusion` dependencies in Cargo.toml. dependencies=( "datafusion" "datafusion-common" "datafusion-expr" "datafusion-functions" "datafusion-optimizer" "datafusion-physical-expr" "datafusion-physical-plan" "datafusion-sql" "datafusion-substrait" ) # Expected revision hash from Cargo.toml expected_hash="d7bda5c9b762426e81f144296deadc87e5f4a0b8" # Function to get the commit hash for a specific revision get_commit_hash() { local repo_url=$1 local revision=$2 git ls-remote $repo_url $revision | awk '{print $1}' } # Check each dependency for dep in "${dependencies[@]}"; do repo_url="https://github.com/waynexia/arrow-datafusion.git" commit_hash=$(get_commit_hash $repo_url $expected_hash) echo "$dep: $commit_hash" doneLength of output: 4966
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (3)
- Cargo.toml (1 hunks)
- src/query/src/datafusion.rs (2 hunks)
- src/query/src/dist_plan/planner.rs (4 hunks)
Additional comments not posted (5)
src/query/src/dist_plan/planner.rs (2)
140-147: LGTM! Direct optimization of the input logical plan.The changes to optimize the input logical plan directly without using an explicit
Analyzerare approved.
Line range hint
377-411: LGTM! Improved handling ofExplainplans andMergeScanoptimization.The changes to take the optimized plan by reference and the improved handling of
Explainplans andMergeScanoptimization are approved.src/query/src/datafusion.rs (3)
51-51: LGTM! Added import forMergeScanLogicalPlan.The addition of the import for
MergeScanLogicalPlanis approved.
377-411: LGTM! Improved handling ofExplainplans andMergeScanoptimization.The changes to handle
Explainplans and optimize logical plans forMergeScanare approved.
395-407: LGTM! Skipped optimization forMergeScanplans.The changes to skip optimization for
MergeScanplans are approved.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4258 +/- ##
==========================================
- Coverage 85.18% 84.91% -0.27%
==========================================
Files 1060 1060
Lines 189069 189092 +23
==========================================
- Hits 161057 160571 -486
- Misses 28012 28521 +509 |
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
!!! DO NOT LEAVE THIS BLOCK EMPTY !!!
Please explain IN DETAIL what the changes are in this PR and why they are needed:
Checklist
Summary by CodeRabbit
Updates
Enhancements