-
Notifications
You must be signed in to change notification settings - Fork 915
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
Adding support for equi-join on struct #7720
Adding support for equi-join on struct #7720
Conversation
…athermap-based-join-apis
…athermap-based-join-apis
…athermap-based-join-apis
This is now ready for review. |
Not sure what you mean by this. Please clarify with readers of the changelog in mind, since this will get pasted into it. |
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7720 +/- ##
===============================================
+ Coverage 81.86% 82.65% +0.78%
===============================================
Files 101 103 +2
Lines 16884 17524 +640
===============================================
+ Hits 13822 14484 +662
+ Misses 3062 3040 -22
Continue to review full report at Codecov.
|
Am I correct in thinking that only the first line is used in the changelog? I have updated the comment to have a one-liner for what the change is on a high level and then detail for reviewers below that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprisingly simple. Does flatten_nested_columns
throw an exception if it encounters a list column?
It marches the table columns looking for structs. Doesn’t complain about lists.
|
So if you had this:
it would flatten to
|
@nvdbaranec I am working on templatized code for |
@nvdbaranec I just coded this up as a quick test and we get "C++ exception with description "cuDF failure at: ../../../../include/cudf/table/row_operators.cuh:363: Attempted to compare elements of uncomparable types." thrown. This is the same error as passing a list by itself to join. |
Nope, not correct. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is such a neat little PR.
@gpucibot merge |
@gpucibot merge |
Adds support for equijoin on structs.
This PR is leveraging the struct PR and the rewrite for join API. It enables equijoin on structs by flattening the struct for the hash calculation.
closes #7543