-
Notifications
You must be signed in to change notification settings - Fork 285
chore: Cast module refactor boolean module #3491
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
base: main
Are you sure you want to change the base?
Conversation
|
@andygrove , @comphead , This is the first part of refactoring out cast ops by data type (and move common functions to utilities). I also went ahead and added tests and benchmarks. Subsequent PRs will follow a similar approach with refactor and all the cast ops are static dispatch with room for additional standardization in the future |
|
CI passed. I believe the PR is ready for your review @andygrove , @comphead |
|
Rebased with main and moved cast_boolean_to_decimal to boolean cast file |
|
Thanks @coderfender. Could you fill in the PR description to cover what is included in this PR, since it looks like a combination of functional changes, refactoring, tests, and benchmarks. I am wondering if would be better to just do the refactoring in this PR and then follow up with a separate PR for the functional changes? |
|
@andygrove , thank you for the review. Flow on main branch :
Feature
Let me create a new PR just for the functional changes / bug fixes to keep changes isolated. However, it might be easier to first merge the functional changes and then continue refactoring IMO |
Which issue does this PR close?
Part 1 of #3459
Rationale for this change
Cast module is currently 3700 LOC and the goal is to refactor this into separate files based on datatype
What changes are included in this PR?
Functional changes :
can_cast_to_booleanwould not support cast toUtf8but the matches macro did. This was mostly due to code duplication among various modules. Alsocan_cast_from_booleanto make it consistentRefactoring Changes
(From) Boolean refactor:
Code :
Unit tests:
Benchmarks:
Created new benchmarks to assess performance
Utils.rs
is_identity_cast() [NEW] , spark_cast_postprocess(), cast_overflow(), invalid_value()toutils.rsHow are these changes tested?