kosiew commented on code in PR #25836:
URL: https://github.com/apache/datafusion/pull/25836#discussion_r4333985661
##########
datafusion/spark/benches/slice.rs:
##########
@@ -115,6 +116,19 @@ fn array_slice_benchmark(
<Arc<Field>>::from(Field::new(format!("arg_{idx}"),
arg.data_type(), true))
})
.collect::<Vec<_>>();
+ // Apply planner coercion (including ListView to List) before timing the
UDF.
Review Comment:
Could we label the ListView cases as `ListView -> List (coercion excluded)`?
Coercion happens before timing, so these measure execution on List rather than
native ListView or conversion cost. Including both types would distinguish them
from existing List cases.
##########
datafusion/functions/benches/string_expressions/repeat.rs:
##########
@@ -215,7 +225,11 @@ fn criterion_benchmark(c: &mut Criterion) {
|b| {
b.iter(|| {
let args_cloned = args.clone();
- black_box(invoke_repeat_with_args(args_cloned,
repeat_times))
+ black_box(
+ invoke_repeat_with_args(args_cloned).expect_err(
Review Comment:
Optional: could we check once outside the timed loop that the overflow error
contains `string size overflow on repeat`? `expect_err` also accepts unrelated
errors. The production regression test already checks this diagnostic, so this
is only a smoke-check improvement.
--
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]
