lakshaydevoo-arch commented on code in PR #27008:
URL: https://github.com/apache/datafusion/pull/27008#discussion_r4455929424
##########
datafusion/common/benches/with_hashes.rs:
##########
@@ -160,6 +160,33 @@ fn criterion_benchmark(c: &mut Criterion) {
do_hash_test(b, &arrays);
});
}
+
+ if name == "dictionary_utf8_int32" {
+ let dict = array
+ .as_any()
+ .downcast_ref::<DictionaryArray<Int32Type>>()
+ .expect("Expected Int32 dictionary");
+
+ for null_percent in [50, 97] {
+ let nullable_array = with_null_keys(dict, null_percent as f32
/ 100.0);
+
+ c.bench_function(
+ &format!("{name}: single, {null_percent}% null keys"),
+ |b| do_hash_test(b, std::slice::from_ref(&nullable_array)),
+ );
+ c.bench_function(
+ &format!("{name}: multiple, {null_percent}% null keys"),
+ |b| {
+ let arrays = vec![
+ nullable_array.clone(),
+ nullable_array.clone(),
+ nullable_array.clone(),
+ ];
+ do_hash_test(b, &arrays);
+ },
+ );
+ }
+ }
Review Comment:
thanks for the suggestion and clarification! i will update the condition to
use the concrete array type and add a comment explaining that the additional
null-density cases intentionally apply to future Int32 dictionary benchmarks as
well
--
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]
