Strong index types for equality comparator #10883
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…p both weakly and strongly typed row comparators.
Co-authored-by: Nghia Truong <ttnghia@users.noreply.github.com>
Co-authored-by: Jake Hemstad <jhemstad@nvidia.com>
… flipping tables.
bdice
reviewed
May 18, 2022
ajschmidt8
approved these changes
May 18, 2022
bdice
reviewed
May 18, 2022
ttnghia
force-pushed
the
strong_typed_index
branch
from
d1a5b7b
to
c51b053
Compare
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
|
@gpucibot merge |
|
@ttnghia the policy is for 2 cpp approvals before merging a PR. |
|
Wait, I had only 1 cpp approval? Sorry I missed that, merged after seeing 2 approvals. Will pay more attention next time. |
jrhemstad
reviewed
May 19, 2022
Comment on lines
+1011
to
+1012
| return strong_index_comparator_adapter<device_row_comparator<Nullate>>{ | ||
| device_row_comparator<Nullate>(nullate, *d_left_table, *d_right_table, nulls_are_equal)}; |
There was a problem hiding this comment.
fyi, CTAD is your friend.
return strong_index_comparator_adapter{device_row_comparator(nullate, *d_left_table, *d_right_table, nulls_are_equal)};
jrhemstad
reviewed
May 19, 2022
Comment on lines
+39
to
+48
| // Create a (structs) column_view of one row having children given from the input scalar. | ||
| auto const needle_tv = static_cast<struct_scalar const*>(&needle)->view(); | ||
| auto const needle_as_col = | ||
| column_view(data_type{type_id::STRUCT}, | ||
| 1, | ||
| nullptr, | ||
| nullptr, | ||
| 0, | ||
| 0, | ||
| std::vector<column_view>{needle_tv.begin(), needle_tv.end()}); |
There was a problem hiding this comment.
We should probably add overloads of the comparators that take a scalar for the lhs or rhs that do this automaticaly.
There was a problem hiding this comment.
I'm glad you mentioned this. I was thinking that would be a helpful next step as well, but I hadn't yet spent the time to identify how many times that pattern occurs.
There was a problem hiding this comment.
This would be an interesting follow up PR.
There was a problem hiding this comment.
Filed an issue: #10892
There was a problem hiding this comment.
Also filed a related FEA issue: #10893
This was referenced May 20, 2022
rapids-bot bot
pushed a commit
that referenced
this pull request
Aug 17, 2022
This extends the `cudf::contains` API to support nested types (lists + structs) with arbitrarily nested levels. As such, `cudf::contains` will work with literally any type of input data. In addition, this fixes null handling of `cudf::contains` with structs column + struct scalar input when the structs column contains null rows at the top level while the scalar key is valid but all nulls at children levels. Closes: #8965 Depends on: * #10730 * #10883 * #10802 * #10997 * NVIDIA/cuCollections#172 * NVIDIA/cuCollections#173 * #11037 * #11356 Authors: - Nghia Truong (https://github.com/ttnghia) - Devavret Makkar (https://github.com/devavret) - Bradley Dice (https://github.com/bdice) - Karthikeyan (https://github.com/karthikeyann) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Bradley Dice (https://github.com/bdice) - Yunsong Wang (https://github.com/PointKernel) URL: #10656
rapids-bot bot
pushed a commit
that referenced
this pull request
Aug 22, 2022
This extends the `lists::contains` API to support nested types (lists + structs) with arbitrarily nested levels. As such, `lists::contains` will work with literally any type of input data. In addition, the related implementation has been significantly refactored to facilitate adding new implementation. Closes #8958. Depends on: * #10730 * #10883 * #10999 * #11019 * #11037 Authors: - Nghia Truong (https://github.com/ttnghia) - Bradley Dice (https://github.com/bdice) Approvers: - MithunR (https://github.com/mythrocks) - Bradley Dice (https://github.com/bdice) URL: #10548
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds strong index types for equality comparator, along with #10730 to unblock #10548, #10656, and several others nested type feature requests.