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

Handle nullptr return value from bitmask_or in distinct_count #13590

Merged
merged 2 commits into from
Jun 21, 2023

Conversation

wence-
Copy link
Contributor

@wence- wence- commented Jun 20, 2023

Description

If nulls should compare equal then we want to filter out rows for which all column entries are invalid (bitmask not set). If any column is not nullable, then bitmask_or returns an empty bitmask buffer (and a null count of zero) indicating that the returned bitmask is fully valid. When passed as a predicate to insert_if we get a null pointer dereference.

To avoid this, only run predicated insertion if the null count returned from bitmask_or is positive (which guarantees that the validity bitmask exists). This also avoids running predicated insertion when the predicate is always true.

Closes #13576.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

If nulls should compare equal then we want to filter out rows for
which all column entries are invalid (bitmask not set). If any column
is not nullable, then bitmask_or returns an empty bitmask buffer (and
a null count of zero) indicating that the returned bitmask is fully
valid. When passed as a predicate to insert_if we get a null pointer
dereference.

To avoid this, only run predicated insertion if the null count
returned from bitmask_or is positive (which guarantees that the
validity bitmask exists). This also avoids running predicated
insertion when the predicate is always true.

Closes rapidsai#13576.
@wence- wence- requested a review from a team as a code owner June 20, 2023 13:42
@wence- wence- requested review from mythrocks and ttnghia June 20, 2023 13:42
@wence- wence- added bug Something isn't working non-breaking Non-breaking change labels Jun 20, 2023
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Jun 20, 2023
@revans2
Copy link
Contributor

revans2 commented Jun 20, 2023

I ran my tests with this patch and I saw no crashes for any of TPCDS, so I am happy with it.

@wence-
Copy link
Contributor Author

wence- commented Jun 20, 2023

I ran my tests with this patch and I saw no crashes for any of TPCDS, so I am happy with it.

Thanks for confirming!

Copy link
Member

@PointKernel PointKernel left a comment

Choose a reason for hiding this comment

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

@wence- thanks for helping fix the issue!

LGTM

@wence-
Copy link
Contributor Author

wence- commented Jun 21, 2023

/merge

@rapids-bot rapids-bot bot merged commit 14dc018 into rapidsai:branch-23.08 Jun 21, 2023
@wence- wence- deleted the wence/fix/13576 branch June 21, 2023 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] cudaErrorIllegalAddress on distinct_count and nunique for reduce
5 participants