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

Remove probe-time null equality parameters in cudf::hash_join #10260

Merged
merged 4 commits into from
Feb 10, 2022

Conversation

PointKernel
Copy link
Member

Closes #9155

This PR removes the probe-time cudf::null_equality parameter in cudf::hash_join to avoid potential mismatching bugs between building and probing a hash join object.

@PointKernel PointKernel added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Feb 9, 2022
@PointKernel PointKernel requested a review from a team as a code owner February 9, 2022 23:23
@PointKernel PointKernel self-assigned this Feb 9, 2022
@codecov
Copy link

codecov bot commented Feb 10, 2022

Codecov Report

Merging #10260 (d888d5b) into branch-22.04 (a7d88cd) will increase coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head d888d5b differs from pull request most recent head ba3fdd8. Consider uploading reports for the commit ba3fdd8 to get more accurate results

Impacted file tree graph

@@              Coverage Diff              @@
##           branch-22.04   #10260   +/-   ##
=============================================
  Coverage         10.42%   10.43%           
=============================================
  Files               119      122    +3     
  Lines             20603    20583   -20     
=============================================
- Hits               2148     2147    -1     
+ Misses            18455    18436   -19     
Impacted Files Coverage Δ
python/cudf/cudf/_fuzz_testing/fuzzer.py 0.00% <0.00%> (ø)
python/cudf/cudf/_fuzz_testing/io.py 0.00% <0.00%> (ø)
python/cudf/cudf/_fuzz_testing/main.py 0.00% <0.00%> (ø)
python/cudf/cudf/_version.py 0.00% <0.00%> (ø)
python/cudf/cudf/comm/gpuarrow.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/_base_index.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/datetime.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/methods.py 0.00% <0.00%> (ø)
... and 48 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d162b4...ba3fdd8. Read the comment docs.

Copy link
Contributor

@codereport codereport left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@jlowe jlowe added breaking Breaking change and removed non-breaking Non-breaking change labels Feb 10, 2022
@jlowe
Copy link
Member

jlowe commented Feb 10, 2022

@PointKernel this is a breaking change, as the public API of cudf::hash_join changed. This is also noted by the broken Java build using that API.

I've updated the Java build for this breaking change in #10268. Feel free to cherry-pick that commit to this PR, or we can merge the other PR after this one is merged (Java CI failures are currently non-blocking).

@PointKernel
Copy link
Member Author

@jlowe Thanks for doing the java changes. I was replying to your java follow-up PR and then noticed your message here. If you don't mind, I will merge your java commits here to make sure CI tests can pass.

@github-actions github-actions bot added the Java Affects Java cuDF API. label Feb 10, 2022
@PointKernel PointKernel requested a review from a team February 10, 2022 16:43
Copy link
Contributor

@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.

Approving the java/JNI code

@PointKernel
Copy link
Member Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 2741e6b into rapidsai:branch-22.04 Feb 10, 2022
@PointKernel PointKernel deleted the remove-probe-null-equal branch February 10, 2022 18:26
@vyasr vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuDF (Java) Reviewer labels Feb 23, 2024
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 4 - Needs Review Waiting for reviewer to review or respond breaking Breaking change bug Something isn't working Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] cudf::hash_join accepts null equality parameter at probe time
5 participants