neilconway commented on code in PR #21064:
URL: https://github.com/apache/datafusion/pull/21064#discussion_r2966436083
##########
datafusion/functions-aggregate/src/approx_distinct.rs:
##########
@@ -212,8 +212,19 @@ where
impl Accumulator for StringViewHLLAccumulator {
fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> {
let array: &StringViewArray = downcast_value!(values[0],
StringViewArray);
- // flatten because we would skip nulls
- self.hll.extend(array.iter().flatten());
+
+ // When all strings are stored inline in the StringView (≤ 12 bytes),
+ // hash the raw u128 view directly instead of materializing a &str.
+ if array.data_buffers().is_empty() {
+ for (i, &view) in array.views().iter().enumerate() {
+ if !array.is_null(i) {
Review Comment:
I took a look at this, but I didn't see an obvious way to reuse/generalize
this code?
I played around with accessing `data_buffers` directly (similar to how
`hash_string_view_array_inner` does it) and computing the hash there for
out-of-line strings (I pushed a commit for this), but it didn't seem like a
huge win (so I pushed a revert for it).
--
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]