sunchao commented on code in PR #4338:
URL: https://github.com/apache/datafusion-comet/pull/4338#discussion_r4176032463


##########
native/core/src/execution/jni_api.rs:
##########
@@ -580,6 +581,7 @@ fn register_datafusion_spark_function(session_ctx: 
&SessionContext) {
     
session_ctx.register_udf(ScalarUDF::new_from_impl(SparkDateSub::default()));
     
session_ctx.register_udf(ScalarUDF::new_from_impl(SparkFromUtcTimestamp::default()));
     
session_ctx.register_udf(ScalarUDF::new_from_impl(SparkLastDay::default()));
+    
session_ctx.register_udf(ScalarUDF::new_from_impl(SparkMakeDtInterval::default()));

Review Comment:
   [P2] Preserve valid intervals when intermediate minutes exceed 32 bits. With 
`spark.comet.expression.MakeDTInterval.allowIncompatible=true`, a column-backed 
`SELECT make_dt_interval(d, h, m) FROM t` containing `(1491309, 0, 
-2147483648)` should return `78720000000` microseconds, or 21 hours 52 minutes. 
The registered kernel instead returns `NULL`: it computes `days * 24 * 60` 
using checked `i32` arithmetic before adding the negative minutes. Spark's 
64-bit operations do not overflow for this input. This silently discards valid 
results and differs from the documented case where Spark throws on overflow. 
Use wider intermediate arithmetic in the kernel, or retain Spark fallback for 
affected inputs.
   
   Evidence: Verified datafusion-spark 53.1.0 against the exact-head Cargo.lock 
checksum and compiled its unchanged `make_interval_dt_nano` body. `(1491309, 0, 
-2147483648, 0.0)` returned `None`, which the kernel converts to a null row. 
Spark 4.1.3 `MakeDTInterval` with bound column references returned 
`78720000000` through both interpreted evaluation and generated 
UnsafeProjection. The simpler `(1491309, 0, 0, 0.0)` also returned None 
natively versus Spark's `128849097600000000`. Sources across all five supported 
profiles accumulate 64-bit microseconds. These are component reproductions, not 
an end-to-end Comet run.



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