Fix inconsistency when hashing two tables in cudf::detail::contains
#11284
Conversation
ttnghia
added
bug
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
This fix looks good - it makes sense and I wish I'd caught this in my initial review. :) @ttnghia Would you be able to write a test that catches this? (I see it was caught by Spark integration tests but I'd like a test in libcudf.)
Yeah the bug seems caused by a not very intuitive reason. |
|
@gpucibot merge |
|
@pxLi Please don't merge C++ PRs unless they have at least 2 approvals. |
sry my bad, I did not realize this rule. I will take it in mind in the future~ |
This is very similar to #11284, which fixes a bug when only one input table has nulls while the other doesn't. This is due to the new experimental hasher producing different hash values depending on an input flag `has_nulls`. In order to properly use it, `has_nulls` must be computed by checking all the possible input tables, or set to a constant value (`true`). Closes: * #13109 Authors: - Nghia Truong (https://github.com/ttnghia) - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Divye Gala (https://github.com/divyegala) - Yunsong Wang (https://github.com/PointKernel) URL: #13120
When hashing elements in a column, the nullable information needs to be taken into account. This could produce different results for the hash values of the same elements if the nullable condition is changed. As such, whenever we need to compute hashing for more than one column/table in the same operation, we need to take into account the nullable of all columns/tables, not just the one that is being hashed.
This PR fixes a bug when hashing two tables using different nullable values in
cudf::detail::contains, which led to incorrect results when there is only one nullable table in the input.