-
Notifications
You must be signed in to change notification settings - Fork 169
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
chore: Use enum to represent CAST eval_mode in expr.proto #361
Comments
Thank you for the issue description. I would like to work on this issue. If no one else is working on it. |
Definitely yes. Thanks @prashantksharma |
I suppose for this issue it is okay to violate the rule to not modify a field in a protobuf message because we haven't published anything yet. |
Yes, absolutely. |
@andygrove , cc: @viirya Minor Query before opening PRSummary:
Minor Query: |
On the Rust side you will need a |
@prashantksharma also, feel free to create a draft PR as it can be easier for maintainers to make suggestions on the PR |
Thank you so much for your feedback:
I will make necessary changes based on my understanding of the above comment by you. And, Thank you for your suggesting creation of draft PR, after making necessary updates, I will do that. |
@andygrove cc: @viirya I have opended a draft PR. I have tested the changes using
Details on PR message:#415 |
* Fixes Issue #361: Use enum to represent CAST eval_mode in expr.proto * Update expr.proto and QueryPlanSerde.scala for handling enum EvalMode for cast message * issue 361 fixed type issue for eval_mode in planner.rs * issue 361 refactored QueryPlanSerde.scala for enhanced type safety and localization compliance, including a new string-to-enum conversion function and improved import organization. * Updated planner.rs, expr.proto, QueryPlanSerde.scala for enum support * Update spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala --------- Co-authored-by: Prashant K. Sharma <[email protected]> Co-authored-by: Andy Grove <[email protected]>
* Fixes Issue apache#361: Use enum to represent CAST eval_mode in expr.proto * Update expr.proto and QueryPlanSerde.scala for handling enum EvalMode for cast message * issue 361 fixed type issue for eval_mode in planner.rs * issue 361 refactored QueryPlanSerde.scala for enhanced type safety and localization compliance, including a new string-to-enum conversion function and improved import organization. * Updated planner.rs, expr.proto, QueryPlanSerde.scala for enum support * Update spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala --------- Co-authored-by: Prashant K. Sharma <[email protected]> Co-authored-by: Andy Grove <[email protected]>
What is the problem the feature request solves?
When I added
eval_mode
to theCast
message inexpr.proto
I defined it as a string but it should really be an enum.Describe the potential solution
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: