jayzhan211 commented on code in PR #25218:
URL: https://github.com/apache/datafusion/pull/25218#discussion_r3995580659
##########
datafusion/physical-expr-common/src/binary_map.rs:
##########
@@ -511,12 +511,21 @@ where
else {
// Check if the value is already present in the set
let entry = self.map.find_mut(hash, |header| {
- // compare value if hashes match
- if header.hash != hash {
+ // Compare the value only when the hashes match and the
+ // existing entry is itself a long value. The length check
+ // is what makes reading the buffer below sound: a short
+ // entry keeps its bytes inlined in `offset_or_inline`
+ // rather than an offset, so on a hash collision between a
+ // short and a long value, `header.range()` would be built
+ // from those bytes and point outside the buffer.
+ if header.hash != hash || header.len != value_len {
Review Comment:
Note that we check length for short+short case but didn't consider
short+long case
--
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]
