apache / apache/datafusion-comet

[EPIC] ANSI mode audit follow-ups (vs Spark 4.1.1)

Open
#5,078 1 comment 2 reactions 1 assignee Claimed by @coderfender View on GitHub
area:expressions bug EPIC priority:medium
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 6h
Merged PRs (30d)
190

Description

### 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_*`).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.