pitrou commented on code in PR #49304:
URL: https://github.com/apache/arrow/pull/49304#discussion_r3111062471


##########
cpp/src/arrow/compute/kernels/vector_rank.cc:
##########
@@ -55,10 +57,14 @@ void MarkDuplicates(const NullPartitionResult& sorted, 
ValueSelector&& value_sel
 
   // Process nulls
   if (sorted.nulls_end != sorted.nulls_begin) {
-    // TODO this should be able to distinguish between NaNs and real nulls 
(GH-45193)
     auto it = sorted.nulls_begin;
+    bool prev_is_null = is_null_selector(*it);
     while (++it < sorted.nulls_end) {
-      *it |= kDuplicateMask;
+      bool curr_is_null = is_null_selector(*it);

Review Comment:
   By the way, we know that, after sorting, nulls are clustered either before 
or either NaNs, so we could take advantage of that to avoid looking the null 
bitmap for each element. But I think the current solution is still good enough.



-- 
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]

Reply via email to