andygrove opened a new issue, #5074: URL: https://github.com/apache/datafusion-comet/issues/5074
### What is the problem the feature request solves? The scalar-function wiring makes it easy to silently lose ANSI semantics, and it has already happened once (`MakeDecimal`). Two independent mechanisms both fail "open" toward non-ANSI behavior: 1. `CometScalarFunction.convert` uses the plain `scalarFunctionExprToProto` helper, which hardcodes `builder.setFailOnError(false)` (`spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:1070-1075`). Any ANSI-sensitive Spark expression wired via plain `CometScalarFunction` drops the flag at serialization time. 2. On the native side, `create_comet_physical_fun` threads `fail_on_error` only into the hand-written match arms; the catch-all `_ => registry.udf(fun_name)` (`native/spark-expr/src/comet_scalar_funcs.rs:240`) drops it for every registry-resolved UDF, which includes all functions wired in from the datafusion-spark crate. `create_comet_physical_fun_with_eval_mode` also defaults `fail_on_error.unwrap_or(false)`. Today no wired datafusion-spark function is ANSI-sensitive (verified against Spark 4.1.1: date_add, date_sub, last_day, from_utc_timestamp, to_utc_timestamp, factorial, chr, substring, space, luhn_check, url_encode, hex, rint, etc. never consult ANSI), so there is no live gap besides `MakeDecimal` (tracked separately). But the failure mode is silent and will recur as more expressions are wired. ### Describe the potential solution Options, not mutually exclusive: - Make `scalarFunctionExprToProtoWithReturnType` (the only helper that can set `fail_on_error`) reject or assert when the native registration cannot receive the flag, or conversely make the native side return an error when `fail_on_error=Some(true)` reaches the registry catch-all instead of silently ignoring it. - Add a debug assertion or serde-level check: expressions whose Spark class has a `failOnError`/`evalMode` field must not be registered via plain `CometScalarFunction`. - Document the constraint in the contributor guide for wiring datafusion-spark functions. ### Additional context Found by an ANSI-mode audit of all native expressions against Spark 4.1.1. The audit relied on name-based variant selection (`parse_url` vs `try_parse_url`, `url_decode` vs `try_url_decode`) as the sound pattern for ANSI-sensitive scalar functions; a guard would enforce that pattern. -- 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]
