alamb commented on code in PR #22172:
URL: https://github.com/apache/datafusion/pull/22172#discussion_r3243710520
##########
datafusion/physical-expr-common/src/binary_view_map.rs:
##########
@@ -389,6 +403,17 @@ where
}
}
+ /// Append an already-computed inline view (len <= 12) directly, bypassing
+ /// buffer allocation. The caller guarantees that `view` is a valid inline
+ /// ByteView (length field in the low 32 bits is <= 12).
+ ///
+ /// Returns the view that was stored (identical to the argument).
+ fn append_inline_view(&mut self, view: u128) -> u128 {
Review Comment:
Given this assumption I think this function should be marked as `unsafe` (I
think it is correct, but the compiler can't verify it)
```suggestion
unsafe fn append_inline_view(&mut self, view: u128) -> u128 {
```
Then this will force us to annotate the callsite with `unsafe` as well where
it can be more easily verified that the input was a valid inline view so this
function is safe
##########
datafusion/physical-expr-common/benches/binary_view_map_insert.rs:
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
While this benchmark is cool, I am not sure we need to commit it to the repo
as I think its utility will be low after this PR has been merged (I think it is
unlikely to get run regularly)
--
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]
