-
Notifications
You must be signed in to change notification settings - Fork 433
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: logical plans in writer #3141
refactor: logical plans in writer #3141
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3141 +/- ##
==========================================
+ Coverage 71.88% 71.92% +0.03%
==========================================
Files 134 134
Lines 43479 43431 -48
Branches 43479 43431 -48
==========================================
- Hits 31257 31236 -21
+ Misses 10201 10165 -36
- Partials 2021 2030 +9 ☔ View full report in Codecov by Sentry. |
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.
Love us adopting logical plans more and more!!
Will give this a more thorough read later on, just looking at the changes alone w/o context - do we want to pass around a DataFrame
or could we just use the LogicalPlan
directly without the attached session?
@roeap the way I was thinking about it, internally we can use dataframes since we use them directly with DF API, and the input for the builder would just be a plan. I was thinking yesterday to also make the schema evolution into a custom logical plan node. Any thoughts on this? |
Most everything should become a logical plan node at some point. I know that's much easier said than done, but still. |
Signed-off-by: Ion Koutsouris <15728914+ion-elgreco@users.noreply.github.com>
58623e6
to
ee6a91b
Compare
Description
We were still using physical plans as starting point, we now only take in logical plans and pass them around. This solves also issues with physical type coercion, since logical type coercion is more flexible.
This is a breaking change though since folks who use .
with_execution_plan
now how to provide a logical plan instead of a physical plan.Related Issue(s)