chore: Cast module refactor boolean module#3491
chore: Cast module refactor boolean module#3491coderfender wants to merge 8 commits intoapache:mainfrom
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 |
19d5899 to
b9d75d5
Compare
|
@andygrove , thank you for removing dead cast code in comet. Given that we removed dead cast code, I believe this PR is ready for a review . I was wondering if you would prefer splitting the PR into smaller PRs (one for benchmarks, another for tests, and another for utils) or would you think we can go ahead with this one PR ? |
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?