taepper commented on code in PR #50248:
URL: https://github.com/apache/arrow/pull/50248#discussion_r3629191919
##########
cpp/src/arrow/compute/kernels/vector_sort_internal.h:
##########
@@ -106,382 +109,337 @@ int CompareTypeValues(Value&& left, Value&& right,
SortOrder order,
}
template <typename IndexType>
-struct GenericNullPartitionResult {
- IndexType* non_nulls_begin;
- IndexType* non_nulls_end;
- IndexType* nulls_begin;
- IndexType* nulls_end;
-
- IndexType* overall_begin() const { return std::min(nulls_begin,
non_nulls_begin); }
+struct GenericNullLikePartition {
+ std::span<IndexType> non_null_like_range;
+ std::span<IndexType> nan_range;
+ std::span<IndexType> null_range;
- IndexType* overall_end() const { return std::max(nulls_end, non_nulls_end); }
+ IndexType* non_null_like_begin() const { return non_null_like_range.data(); }
+ IndexType* non_null_like_end() const {
+ return non_null_like_range.data() + non_null_like_range.size();
+ }
+ IndexType* nan_begin() const { return nan_range.data(); }
+ IndexType* nan_end() const { return nan_range.data() + nan_range.size(); }
+ IndexType* null_begin() const { return null_range.data(); }
+ IndexType* null_end() const { return null_range.data() + null_range.size(); }
- int64_t non_null_count() const { return non_nulls_end - non_nulls_begin; }
+ IndexType* overall_begin() const {
+ // nans are always in the middle
+ return std::min(non_null_like_begin(), null_begin());
+ }
- int64_t null_count() const { return nulls_end - nulls_begin; }
+ IndexType* overall_end() const {
+ // nans are always in the middle
+ return std::max(non_null_like_end(), null_end());
+ }
- static GenericNullPartitionResult NoNulls(IndexType* indices_begin,
- IndexType* indices_end,
- NullPlacement null_placement) {
- if (null_placement == NullPlacement::AtStart) {
- return {indices_begin, indices_end, indices_begin, indices_begin};
+ // Note that "_begin" is not actually the begin of the stored ranges, but
can be much
+ // smaller. I.e. this function can be and is used when the Partition object
is pointing
+ // into a larger buffer and translate it into another larger buffer at the
same offset
+ template <typename TargetIndexType>
+ GenericNullLikePartition<TargetIndexType> TranslateTo(
+ IndexType* indices_begin, TargetIndexType* target_indices_begin) const {
+ size_t non_null_offset = non_null_like_range.data() - indices_begin;
Review Comment:
I wanted to do exactly that and only then realized that the usage of this
function (`ChunkedArraySorter::SortInternal`) is actually very different:
- an `indices_` vector is allocated once (with size = sum of chunk sizes =
chunked-array-size)
- within `indices_` every chunk's indices are partitioned and stored in
series
- `indices_` is translated to `chunked_indices` using `ChunkedIndexMapper`
- *each `NullLikePartition` is "cast" to the chunked variant using
`TranslateTo`*
the final "casting" uses `indices_.data()` and `chunked_indices.data()`,
even though the actual partitions do not have `indices_.data()` within their
overall range.
That is the reason why I added the comment above the `TranslateTo` method,
but maybe it can be improved?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
