perf: optimisation for date_part with seconds - #23444
Conversation
|
run benchmark date_part |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf-date_part-seconds (a68f33a) to 1e77af8 (merge-base) diff using: date_part File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagedate_part — base (merge-base)
date_part — branch
File an issue against this benchmark runner |
Jefffrey
left a comment
There was a problem hiding this comment.
do we have test coverage for date_part on date types?
| } | ||
|
|
||
| // Fast path for Date32 and Date64 - no seconds | ||
| match array.data_type() { |
There was a problem hiding this comment.
if array.data_type() == &Date32 || array.data_type() == &Date64 {
return Ok(Arc::new(Int32Array::new(
vec![0; array.len()].into(),
array.nulls().cloned(),
)));
}applicable below too
There was a problem hiding this comment.
Now we do - added tests for date32/date64, and it allowed me to catch a bug
There was a problem hiding this comment.
btw my above comment is a way to simplify the code
Which issue does this PR close?
date_partUDF optimisations #23351.Rationale for this change
Benches were added recently in #23350 . Improve and measure some optimisations.
What changes are included in this PR?
seconds_nsandseconds_as_i32. A pretty rare code path.Are these changes tested?
Are there any user-facing changes?