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.

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

Add JNI for strings::repeat_strings [skip ci] #8491

Merged
merged 6 commits into from
Jun 11, 2021

Conversation

ttnghia
Copy link
Contributor

@ttnghia ttnghia commented Jun 10, 2021

This PR adds JNI for strings::repeat_strings.

@ttnghia ttnghia added feature request New feature or request 2 - In Progress Currently a work in progress Java Affects Java cuDF API. Spark Functionality that helps Spark RAPIDS strings strings issues (C++ and Python) non-breaking Non-breaking change labels Jun 10, 2021
@ttnghia ttnghia requested review from jlowe and revans2 June 10, 2021 22:58
@ttnghia ttnghia self-assigned this Jun 10, 2021
@ttnghia ttnghia changed the title Add JNI for strings::repeat_strings Add JNI for strings::repeat_strings [skip ci] Jun 10, 2021
@rapidsai rapidsai deleted a comment from codecov bot Jun 11, 2021
@ttnghia ttnghia added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Jun 11, 2021
@rapidsai rapidsai deleted a comment from codecov bot Jun 11, 2021
@ttnghia ttnghia marked this pull request as ready for review June 11, 2021 14:50
@ttnghia ttnghia requested a review from a team as a code owner June 11, 2021 14:50
@rapidsai rapidsai deleted a comment from codecov bot Jun 11, 2021
java/src/main/java/ai/rapids/cudf/ColumnView.java Outdated Show resolved Hide resolved
java/src/main/java/ai/rapids/cudf/ColumnView.java Outdated Show resolved Hide resolved
java/src/main/java/ai/rapids/cudf/ColumnView.java Outdated Show resolved Hide resolved
*
* Note that this function cannot handle the cases when the size of the output column exceeds
* the maximum value that can be indexed by int type. In such situations, the output result
* is undefined.
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting, could not we just repeat as many times as fit ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bound check should be done on the plugin side to make sure the output is correct. I tried to implement bounds check in cudf C++ but was rejected.

java/src/main/java/ai/rapids/cudf/ColumnView.java Outdated Show resolved Hide resolved
@rapidsai rapidsai deleted a comment from codecov bot Jun 11, 2021
Copy link
Contributor

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM

@ttnghia
Copy link
Contributor Author

ttnghia commented Jun 11, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 709adb1 into rapidsai:branch-21.08 Jun 11, 2021
@ttnghia ttnghia deleted the add_jni_repeat_strings branch July 23, 2021 03:30
rapids-bot bot pushed a commit that referenced this pull request Jul 26, 2021
…eparately by different numbers of times (#8572)

This is a follow up work on #8491 so that cudf's `strings::repeat_strings` fully supports StringRepeat SQL expression in Apache Spark.

This PR also rewrites some existing code, including re-formatting code and changes in doxygen.

Authors:
  - Nghia Truong (https://github.com/ttnghia)

Approvers:
  - Jason Lowe (https://github.com/jlowe)

URL: #8572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team feature request New feature or request Java Affects Java cuDF API. non-breaking Non-breaking change Spark Functionality that helps Spark RAPIDS strings strings issues (C++ and Python)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants