andygrove opened a new issue, #5078:
URL: https://github.com/apache/datafusion-comet/issues/5078

   ### What is the problem the feature request solves?
   
   This epic tracks the follow-ups from a full audit of ANSI mode support 
across all natively-supported Comet expressions, compared against Spark 4.1.1 
source. The audit covered every expression whose runtime behavior depends on 
`failOnError` / `evalMode` / `SQLConf.get.ansiEnabled` in Spark 4.1 
(arithmetic, aggregates, decimal internals, Cast/TryEval, collections, 
datetime, math/string/url), including functions wired in from the 
datafusion-spark crate.
   
   Since Comet no longer has a global ANSI fallback config, ANSI correctness is 
entirely per-expression, and ANSI is the default in Spark 4.x, which raises the 
stakes for every gap below.
   
   ### Findings: correctness bugs (ANSI silently ignored or wrong result)
   
   - [ ] #5065 IntegralDivide: `Long.MinValue div -1` returns wrong value 
instead of throwing under ANSI
   - [ ] #5066 MakeDecimal: `fail_on_error` serialized but discarded natively, 
NULL where Spark throws
   - [ ] #5067 Remainder: Float/Double `% 0` returns NaN under ANSI instead of 
throwing
   - [ ] #5068 Cast boolean to decimal ignores eval mode: throws where Spark 
returns NULL in legacy/try mode
   - [ ] #5070 round on LongType with scale <= -19 returns 0 instead of 
overflowing (both modes)
   
   ### Findings: error fidelity (throw/NULL decision correct, wrong exception 
type or message)
   
   - [ ] #5071 Arithmetic overflow errors: wrong error class for Byte/Short, 
wrong type names, missing `try_` suggestions
   - [ ] #5072 Native ANSI errors raised as Arrow errors bypass SparkError 
conversion (wide decimal, decimal divide, decimal-to-decimal cast)
   - [ ] #5073 next_day and make_date ANSI errors surface as 
CometNativeException instead of Spark exception classes
   
   ### Findings: enhancements and hardening
   
   - [ ] #5069 Cast float/double to decimal: return NULL for NaN/Infinity under 
ANSI (pair is already marked incompatible)
   - [ ] #5074 Guard against silent `fail_on_error` loss in scalar function 
wiring
   - [ ] #5075 DecimalPrecision.promote uses session ANSI flag instead of 
per-expression eval mode (latent)
   - [ ] #5076 Test coverage: re-enable stale ignored ANSI tests, add missing 
ANSI cases, strengthen error-class assertions
   - [ ] #5077 Clean up dead ANSI-related plumbing
   
   ### What the audit verified as correct (no action needed)
   
   - **Collections**: `GetArrayItem` and array `element_at` serialize 
`failOnError`, honor it natively, and reconstruct genuine Spark exceptions with 
error class, parameters, and query context. `GetMapValue` and map `element_at` 
correctly need no ANSI handling (the map-key ANSI error no longer exists in any 
supported Spark version).
   - **Sum / Average** (integral and decimal): eval mode serialized and honored 
natively, including `CheckOverflowInSum` semantics and the mixed partial/final 
guard. Spark 4.1's `NumericEvalContext` is modeled via the 4.1 shim.
   - **Cast**: the string-to-numeric/date/timestamp/boolean/decimal family is 
fully mode-correct, including the `canUseLegacyCastForTryCast` cases where TRY 
intentionally equals LEGACY. Mode-dependent cast legality (date to numeric, int 
to binary, etc. being legal only in legacy mode) is correctly gated in 
`CometCast`. TimeType casts fall back.
   - **JVM codegen dispatch family** (`Pmod`, `BRound`, `Conv`, `Elt`, 
`UnBase64`, `ToNumber`, `GetTimestamp`, `ToUnixTimestamp`, `MakeTimestamp`, 
`TryEval`, and others): ANSI-exact by construction because Spark's own 
generated code runs, with clean fallback when the dispatcher is disabled.
   - **Name-based try variants**: `parse_url`/`try_parse_url` and 
`url_decode`/`try_url_decode` are the sound pattern for ANSI-sensitive scalar 
functions.
   - **datafusion-spark wired functions**: none of the currently wired 
functions (date_add, date_sub, last_day, from_utc_timestamp, to_utc_timestamp, 
factorial, chr, substring, space, luhn_check, url_encode, hex, rint, and the 
rest) is ANSI-sensitive in Spark 4.1, so no ANSI behavior is lost through that 
path today (see #5074 for keeping it that way).
   - **Correct non-support**: `DateAddInterval`, `MakeInterval`, 
`MultiplyInterval`/`DivideInterval` fall back to Spark. Empty `IN ()` ANSI 
semantics are unreachable (the optimizer eliminates them before serde).
   - **Bitwise and/or/xor** hardcode legacy eval mode in Spark and are 
correctly ANSI-independent, as are `Right` (deliberately non-ANSI negation in 
Spark), `RoundCeil`/`RoundFloor`, `Rand`/`Randn`.
   
   ### Related
   
   - #4967 (Spark 4.2 ANSI arithmetic overflow test failures) is plausibly 
explained by the error-fidelity cluster (#5071, #5072).
   - #2855 (remainder-by-zero wording for Spark 4.1+) relates to #5067.
   - #2179 (more types in `try_*`).
   


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