Dandandan commented on code in PR #21064:
URL: https://github.com/apache/datafusion/pull/21064#discussion_r2963993690


##########
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 am wondering, perhaps we can reuse/generalize 
`hash_string_view_array_inner` as well (passing a `quality` hash function 
instead)?
   It has some more optimization for specializing on nulls, etc.



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

Reply via email to