feat: Use enum to represent CAST eval_mode in expr.proto

prashantksharma commented 3 weeks ago

Which issue does this PR close?

Closes #361

Rationale for this change

The updates to expr.proto by using enums instead of strings, which prevents potential errors in type interpretation. Modifications in planner.rs and QueryPlanSerde.scala resolve issues found during compilation and style checks.

What changes are included in this PR?

expr.proto: Updated the eval_mode field from a string type to an enum type (EvalMode), with defined values LEGACY, TRY, and ANSI.
Rust Code: Modified ExprStruct::Cast(expr) in planner.rs to handle eval_mode as integers corresponding to the new protobuf enum. Updated the match statement to directly convert integers to the EvalMode Rust enum and added error handling for invalid integers.
QueryPlanSerde.scala:
- Introduced the stringToEvalMode function, which converts input strings to ExprOuterClass.EvalMode enum values using a case-insensitive match, enhancing our error handling by throwing exceptions for invalid modes.
- Corrected the use of toUpperCase to include Locale.ROOT to avoid locale-sensitive behavior, aligning with internationalization best practices.
- Reorganized imports to comply with project's Scalastyle guidelines, clearly separating Java standard library imports from those specific to the project, which aids in code clarity and maintenance.
- Addressed and resolved Scalastyle violations related to locale-specific string manipulations. Fixed the Scalastyle violation by correcting the usage of toUpperCase without specifying a locale. Reorganized import statements to adhere to the project's coding standards, ensuring that Java standard library imports are correctly grouped.

How are these changes tested?

I have run

make test-rust: No failures.
make test-jvm: No failures.

andygrove commented 1 week ago

@prashantksharma Do you need any assistance with this PR? Let me know if you have questions

prashantksharma commented 6 days ago

@prashantksharma Do you need any assistance with this PR? Let me know if you have questions

@andygrove Apologies, I will definitely finalize this PR based on your comments by May 31st, 2024.

PS: I have been little caught up with office work due to product launch at my current company.

prashantksharma commented 22 hours ago

@andygrove

Thank you so much for your suggestions and comments, and most importantly patience in helping me with this PR.

I have made the necessary changes and pushed to the same branch i.e issue-361-enum-for-eval_mode.

Kindly, let me know if there are any steps to be done for this PR from my side.

apache / datafusion-comet