-
Notifications
You must be signed in to change notification settings - Fork 921
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
Cleanup iterator.cuh
and add fixed point support for scalar_optional_accessor
#10999
Cleanup iterator.cuh
and add fixed point support for scalar_optional_accessor
#10999
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
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.
Why return type const
qualifier is considered redundant ?
return type is marked const
so that the user does not try to modify the dereferenced value.
Look at https://godbolt.org/z/n5MjTxqrf
Ah I see the purpose:
Typically we don't need that, but here we have the iterators naming |
rerun tests |
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.
Thanks, this looks good to me!
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.
LGTM 👍
@gpucibot merge |
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
This PR does the following:
iterator.cuh
, enforcingconst
qualifier for accessors'operator()
.operator()
functions, as they seem trivial: the struct iterators already state clearly their purpose.operator()
functions__device__
callable only: there is no need to call them (and they are never called) from host code.template <bool safe = false>
formake_validity_iterator(scalar const&)
overload, to make it having identical signature as themake_validity_iterator(column_device_view const&)
overload.scalar_optional_accessor
.This unblocks nested types support in
lists::contains
(#10548), which needs fixed point support forscalar_optional_accessor
.