-
Notifications
You must be signed in to change notification settings - Fork 204
[AURON #1833] Refactor from_proto.rs to planner.rs #1843
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
|
|
Sure, will rename the module to auron-planner as suggested. |
0b83e8f to
2bd6aa7
Compare
|
@richox PTAL |
|
Is this associated with the wrong issue? |
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.
Pull request overview
This pull request refactors the physical plan creation logic from from_proto.rs into a more organized planner.rs module. The PR introduces a PhysicalPlanner struct that encapsulates the planning logic and stores a task-level partition_id, which will be used for implementing nondeterministic functions in future work. The refactoring also involves renaming the crate from auron-serde to auron-planner to better reflect its purpose.
Changes:
- Introduced
PhysicalPlannerstruct withpartition_idfield for converting Spark query plans to DataFusion physical plans - Refactored static functions to instance methods on
PhysicalPlannerand convertedTryIntotrait implementation tocreate_planmethod - Renamed crate from
auron-serdetoauron-plannerand moved file fromfrom_proto.rstoplanner.rswith updated macro definitions
Reviewed changes
Copilot reviewed 8 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| native-engine/auron-planner/src/planner.rs | Main refactored file - converts TryInto trait implementation to PhysicalPlanner with create_plan method; all parsing functions now take &self to access partition_id |
| native-engine/auron-planner/src/lib.rs | Updates module name from from_proto to planner and modifies convert_box_required macro to call self.create_plan |
| native-engine/auron-planner/src/error.rs | New error handling module defining PlanSerDeError types and conversions |
| native-engine/auron-planner/proto/auron.proto | New protobuf schema definition for physical plan serialization |
| native-engine/auron-planner/build.rs | New build script for compiling protobuf definitions |
| native-engine/auron-planner/Cargo.toml | Renames crate from auron-serde to auron-planner |
| native-engine/auron/src/rt.rs | Updates to use PhysicalPlanner with partition_id instead of TryInto trait |
| native-engine/datafusion-ext-functions/src/lib.rs | Adds spark_partition_id parameter to create_auron_ext_function |
| native-engine/auron/Cargo.toml | Updates dependency from auron-serde to auron-planner |
| dev/mvn-build-helper/proto/pom.xml | Updates proto source path from auron-serde to auron-planner |
| Cargo.toml | Updates workspace member and dependency from auron-serde to auron-planner |
| Cargo.lock | Reflects crate name change in dependency graph |
Comments suppressed due to low confidence (2)
native-engine/auron-planner/src/planner.rs:1009
- Variable name contains a typo: "pyhsical_sort_expr" should be "physical_sort_expr".
native-engine/auron-planner/src/planner.rs:1045 - Variable name contains a typo: "pyhsical_sort_expr" should be "physical_sort_expr".
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Which issue does this PR close?
Closes #1833
Rationale for this change
Refactor planning logic from
from_proto.rsintoplanner.rsand introducePhysicalPlannerfor converting Spark query plans to DataFusion physical plans, adding support for passing a task-level partition_id during conversion. This partition_id will be used by upcoming implementations of nondeterministic functions.What changes are included in this PR?
Are there any user-facing changes?
How was this patch tested?