andygrove commented on issue #4679: URL: https://github.com/apache/datafusion-comet/issues/4679#issuecomment-4836524182
Verified empirically on Spark 4.1 (ANSI on by default): `make_timestamp` already throws under ANSI for invalid arguments, matching Spark, so this is not a divergence on the current codebase. Root cause of the stale report: `CometMakeTimestamp` is a codegen-dispatch serde, and `spark.comet.exec.scalaUDF.codegen.enabled` defaults to `true`. The dispatcher compiles Spark's own `MakeTimestamp.doGenCode` into the per-batch kernel, so under ANSI it produces the same throw site as Spark, and the JVM-UDF path propagates the exception (`native/spark-expr/src/jvm_udf/mod.rs` `check_exception`) rather than returning NULL. No `failOnError` gating is needed. This was resolved by #4417 (merged 2026-05-26), which added the codegen-dispatch date/time path and the `make_timestamp_ansi.sql` regression test, predating this issue (filed 2026-06-18); the audit that split this out from #4502 did not account for #4417. Confirmed passing: - `make_timestamp_ansi.sql` (month/day/hour out-of-range throw, plus a sentinel `checkSparkAnswerAndOperator` query proving native execution, so the throws are not vacuous Spark fallbacks). - A local probe of the seconds cases this issue called out: `make_timestamp(..., 61)` throws `SecondOfMinute` and `make_timestamp(..., 60.5)` throws the fraction-of-second error, both matching Spark, with the sentinel confirming native execution. Closing as already resolved. -- 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]
