Ensure we pass the has_nulls tparam to mixed_join kernels#16708
Ensure we pass the has_nulls tparam to mixed_join kernels#16708rapids-bot[bot] merged 3 commits intorapidsai:branch-24.10from
Conversation
|
/ok to test |
|
Do we not have a simple gtest for this? |
|
I was able to build our plugin with a cuDF that has this change and it fixes our issue. |
There is code in mixed_join_tests that should test the null path: https://github.com/rapidsai/cudf/blob/branch-24.10/cpp/tests/join/mixed_join_tests.cu#L711, and the not null path: https://github.com/rapidsai/cudf/blob/branch-24.10/cpp/tests/join/mixed_join_tests.cu#L680. I think..., I am not familiar with this code, so I could be missing it. That said, these tests are not failing with or without my change. We have been able to repro it from spark, and its apparent from the linked PR that this was missed. That said I do think we should add a test or figure out why it isn't working. I tried setting |
|
I think it is hard to reproduce a test that "fails" because this is a memory access UB issue rather than a validity issue. |
|
/ok to test |
|
/merge |
Fixes #16706
I'll build/test our stack with this change, but it looks like a typo.
If there's a quick unit test we can add I'd be happy to hear recommendations or for someone else to follow on with such a test.