Jefffrey commented on code in PR #23291:
URL: https://github.com/apache/datafusion/pull/23291#discussion_r3520906622


##########
datafusion/functions-aggregate/benches/approx_distinct.rs:
##########
@@ -231,6 +241,18 @@ fn approx_distinct_benchmark(c: &mut Criterion) {
             })
         });
 
+        // --- Duration benchmarks ---
+        let values = Arc::new(create_duration_array(n_distinct)) as ArrayRef;
+        c.bench_function(&format!("approx_distinct duration {pct}% distinct"), 
|b| {

Review Comment:
   honestly i feel we dont need to add a benchmark for this; it would be good 
to cut down on the benchmarks as having too many results can be too noisy, 
especially as the codepath being tested here would be the same for all 
primitives anyway 🤔 



##########
datafusion/functions-aggregate/src/approx_distinct.rs:
##########
@@ -1039,6 +1054,39 @@ mod tests {
             );
         }
 
+        #[test]
+        fn duration_support_numerical_acc_and_group_acc() {

Review Comment:
   we could probably omit this test since we have the SLT? or is this to handle 
the MAX/MIN cases?



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