nuno-faria commented on code in PR #22911:
URL: https://github.com/apache/datafusion/pull/22911#discussion_r3409472367
##########
datafusion/physical-plan/src/joins/array_map.rs:
##########
@@ -402,8 +425,7 @@ impl ArrayMap {
}
// SAFETY: i is within bounds [0, arr.len())
let key: u64 = unsafe { arr.value_unchecked(i) }.as_();
- let idx = key.wrapping_sub(self.offset) as usize;
- idx < self.data.len() && self.data[idx] != 0
+ self.key_present(key)
Review Comment:
Then here we just need to check if it `is_some()`.
##########
datafusion/physical-plan/src/joins/array_map.rs:
##########
@@ -264,6 +273,16 @@ impl ArrayMap {
)
}
+ /// Returns whether `key` (a raw probe value cast to `u64`) is present in
the
+ /// build side, i.e. maps to a non-empty bucket.
+ #[inline]
+ fn key_present(&self, key: u64) -> bool {
+ let Some(idx) = Self::key_to_index(key, self.offset, self.data.len())
else {
+ return false;
+ };
+ self.data[idx] != 0
+ }
+
Review Comment:
What do you think about having this function return the value if it is != 0
(i.e., `get_value -> Option<u32>`)? This way we can simply call it in the two
changes below, instead of calling `key_to_index` and then checking if it is 0.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
