-
Notifications
You must be signed in to change notification settings - Fork 316
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
refactor: Remove PhysicalPlan trait and use ExecutionPlan directly #3894
refactor: Remove PhysicalPlan trait and use ExecutionPlan directly #3894
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3894 +/- ##
==========================================
- Coverage 85.97% 85.68% -0.30%
==========================================
Files 961 960 -1
Lines 164471 164247 -224
==========================================
- Hits 141409 140738 -671
- Misses 23062 23509 +447 |
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.
LGTM
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.
LGTM.
Does this patch close the two issues linked above?
No. It is a part of them. |
Draft this to avoid an incoming potential conflict |
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Did we fix this test in the main branch?
|
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?
While implementing the
RegionScanner
trait mentioned in #3886, I need to adapt the scanner to anExecutionPlan
of the datafusion. I found that we could remove thePhysicalPlan
trait from the codebase to reduce some indirections.This PR removes the
PhysicalPlan
trait and related adapters:DfPhysicalPlanAdapter
andPhysicalPlanAdapter
.Checklist