-
Couldn't load subscription status.
- Fork 537
refactor: consolidate extension planners #3804
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3804 +/- ##
==========================================
+ Coverage 76.05% 76.07% +0.02%
==========================================
Files 145 145
Lines 45274 45313 +39
Branches 45274 45313 +39
==========================================
+ Hits 34433 34472 +39
Misses 9150 9150
Partials 1691 1691 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Robert Pack <[email protected]>
e87e09c to
48e5ef9
Compare
|
Correct me if I am wrong, but adding all extensions planner together will cause the plan become messy and do multiple metric counts |
The individual planners, while all downcasting to the same In case there are duplicate IDs we would potentially match wrong depending on the first planner that signifies it can plan the extension node (returning |
Yeah, I thought our IDs werent unique but it's been a while that I've touched these extension planners |
same 😆 In a post kernelized world, I do believe we'll see much more of the likes though ... |
Description
We currently create a dedicated session with a dedicated planner in some operations to collect metrics. This creates a situation where - even if we pass a session - we need to recreate the session on the fly, potentially replacing an external planner and thus potentially loosing support for UDFs or other custom nodes.
This PR consolidates the various extension planner we currently have into a single planner to eventually make it easy for us to centralise session creation or allow others to extend their own planner with the delta extension planners.
cc @abhi-airspace-intelligence
Related Issue(s)
part-of: #3799