zanmato1984 commented on code in PR #45217: URL: https://github.com/apache/arrow/pull/45217#discussion_r1912979888
########## cpp/src/arrow/compute/kernels/vector_rank.cc: ########## @@ -28,114 +28,97 @@ namespace { // ---------------------------------------------------------------------- // Rank implementation -template <typename ValueSelector, - typename T = std::decay_t<std::invoke_result_t<ValueSelector, int64_t>>> +// A bit that is set in the sort indices when the value at the current sort index +// is the same as the value at the previous sort index. +constexpr uint64_t kDuplicateMask = 1ULL << 63; + +constexpr bool NeedsDuplicates(RankOptions::Tiebreaker tiebreaker) { + return tiebreaker != RankOptions::First; +} + +template <typename ValueSelector> +void MarkDuplicates(const NullPartitionResult& sorted, ValueSelector&& value_selector) { + using T = decltype(value_selector(int64_t{})); + + // Process non-nulls + if (sorted.non_nulls_end != sorted.non_nulls_begin) { + auto it = sorted.non_nulls_begin; + T prev_value = value_selector(*it); + while (++it < sorted.non_nulls_end) { + T curr_value = value_selector(*it); + if (curr_value == prev_value) { + *it |= kDuplicateMask; Review Comment: Shall we somehow expose the index type of `GenericNullPartitionResult` and `static_assert` somewhere in this function that it is `uint64_t`? ########## cpp/src/arrow/type_traits.h: ########## @@ -1069,6 +1069,13 @@ constexpr bool is_floating(Type::type type_id) { return false; } +/// \brief Check for a physical floating point type +/// +/// This predicate matches floating-point types, except half-float. +constexpr bool is_physical_floating(Type::type type_id) { Review Comment: How about we use `switch cases` in this function and change `is_floating` to call `return is_physical_floating() and type_id == Type::HALF_FLOAT`? -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org