sunchao commented on code in PR #5738:
URL: https://github.com/apache/datafusion-comet/pull/5738#discussion_r3953707492
##########
native/spark-expr/src/datetime_funcs/extract_date_part.rs:
##########
@@ -117,6 +122,38 @@ extract_date_part!(SparkHour, "hour", Hour);
extract_date_part!(SparkMinute, "minute", Minute);
extract_date_part!(SparkSecond, "second", Second);
+/// Spark 4.1 EXTRACT(SECOND FROM TIME): truncate to the input precision and
return
+/// Decimal(8,6). The precision is a literal supplied by Spark's TimeType
lowering.
+pub fn spark_seconds_of_time(args: &[ColumnarValue]) -> Result<ColumnarValue> {
+ let [time, precision] = take_function_args("seconds_of_time", args)?;
+ let precision = match precision {
+ ColumnarValue::Scalar(ScalarValue::Int32(Some(p))) if
(0..=6).contains(p) => *p as u32,
+ _ => {
+ return Err(internal_datafusion_err!(
+ "seconds_of_time requires a literal precision from 0 to 6"
+ ))
+ }
+ };
+ let divisor = 10_i64.pow(9 - precision);
+ let multiplier = 10_i128.pow(6 - precision);
+ let extract = |nanos: i64| i128::from((nanos % 60_000_000_000) / divisor)
* multiplier;
+ match time {
+ ColumnarValue::Array(array) => {
+ let times = as_time64_nanosecond_array(array.as_ref())?;
+ let seconds: Decimal128Array = times.iter().map(|nanos|
nanos.map(extract)).collect();
+ Ok(ColumnarValue::Array(Arc::new(
+ seconds.with_precision_and_scale(8, 6)?,
Review Comment:
### Performance
[P2] Add a focused microbenchmark and results for this new native
`EXTRACT(SECOND FROM TIME)` path before enabling it. The existing `to_time`
benchmarks measure parsing, and this PR supplies correctness tests but no
extraction benchmark or measurements. The new shim replaces the existing Comet
JVM codegen route for eligible expressions, so a Spark-only comparison would
also miss that baseline. Please compare matched Spark, existing Comet
dispatcher, and native runs on column input, including NULLs and representative
TIME precisions, and verify the execution plans. Isolate extraction from
parsing where possible and include the composed `to_time` case. If native is
slower, document the acceptance rationale or a concrete optimization plan.
--
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]
