Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add JNI backend for Spark SQL function conv for (hexa)decimals #1314

Merged
merged 20 commits into from
Aug 17, 2023
Merged

Add JNI backend for Spark SQL function conv for (hexa)decimals #1314

merged 20 commits into from
Aug 17, 2023

Conversation

gerashegalov
Copy link
Collaborator

@gerashegalov gerashegalov commented Aug 3, 2023 •
edited
Loading

Contributes to NVIDIA/spark-rapids#8511

POC supporting form/to radices 10 and 16 leveraging existing libcudf API

Signed-off-by: Gera Shegalov [email protected]

@gerashegalov gerashegalov marked this pull request as draft August 3, 2023 18:11
@gerashegalov gerashegalov changed the base branch from branch-23.08 to branch-23.10 August 3, 2023 18:12
@gerashegalov
clang-format
0cd491a 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov gerashegalov mentioned this pull request Aug 3, 2023
Merged
revans2
revans2 reviewed Aug 3, 2023
View reviewed changes
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally it looks fine. Just a little curious about an error case.

src/main/cpp/src/CastStringJni.cpp Show resolved Hide resolved
@gerashegalov gerashegalov self-assigned this Aug 3, 2023
@gerashegalov gerashegalov marked this pull request as ready for review August 12, 2023 09:49
@gerashegalov gerashegalov changed the title [WIP] Add JNI backend for Spark SQL function conv Add JNI backend for Spark SQL function conv for (hexa)decimals Aug 12, 2023
@gerashegalov
Copy link
Collaborator Author

build

revans2
revans2 reviewed Aug 14, 2023
View reviewed changes
src/main/cpp/src/CastStringJni.cpp Outdated Show resolved Hide resolved
src/main/cpp/src/CastStringJni.cpp Outdated Show resolved Hide resolved
src/main/java/com/nvidia/spark/rapids/jni/CastStrings.java Outdated Show resolved Hide resolved
src/main/cpp/src/CastStringJni.cpp Outdated Show resolved Hide resolved
src/main/cpp/src/CastStringJni.cpp Outdated Show resolved Hide resolved
@gerashegalov gerashegalov mentioned this pull request Aug 15, 2023
Open
revans2
revans2 previously approved these changes Aug 16, 2023
View reviewed changes
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern now is performance. I just want to be sure that the cost of the regular expressions/etc beats the CPU. But that can even wait for the plugin changes if needed, because that is where I actually care about the performance.

@gerashegalov
handling zeroes
022afae 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov
leading zero dec tests
0f7e0a9 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
leading zero hex tests
9ea2651 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov
Copy link
Collaborator Author

@revans2 PTAL, it should be ready since it passes https://github.com/NVIDIA/spark-rapids/pull/8925/files#diff-af93e5faff863709f2c9de84607d8bd79c6cc0e2454b6f982c693c10ad721402R784-R795

@gerashegalov gerashegalov mentioned this pull request Aug 17, 2023
Open
4 tasks
@gerashegalov gerashegalov merged commit 72cb837 into NVIDIA:branch-23.10 Aug 17, 2023
1 check passed
@gerashegalov gerashegalov deleted the conv_jni branch August 17, 2023 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@revans2 revans2 revans2 approved these changes

Assignees

@gerashegalov gerashegalov

Labels
feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gerashegalov @revans2 @sameerz