apache / apache/datafusion-comet

Clean up dead ANSI-related plumbing (CometEvalMode helpers, inert allow_incompat on casts)

Open
#5,077 1 comment 0 reactions 1 assignee Claimed by @sam-1112 View on GitHub
area:expressions enhancement
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 4h
Merged PRs (30d)
198

Description

### What is the problem the feature request solves?

An ANSI-mode audit against Spark 4.1.1 turned up a few pieces of dead or misleading ANSI-related plumbing worth cleaning up:

- `CometEvalMode.fromBoolean` and `CometEvalMode.fromString` (`spark/src/main/scala/org/apache/comet/expressions/CometEvalMode.scala:35-41`) have no callers.
- `SparkCastOptions::allow_incompat` is serialized into every cast (`native/spark-expr/src/conversion_funcs/cast.rs:134`) but never read by `cast_array`; the only consumer is `native/core/src/parquet/schema_adapter.rs`. For casts, `allowIncompatible` is purely a plan-time gate in `CometCast`. Either drop the field from the cast proto/serde path or document that it is plan-time-only, so readers do not assume the native cast changes behavior based on it.
- `SparkError::IntegralDivideOverflow` and `SparkError::BinaryArithmeticOverflow` (`native/common/src/error.rs:72-73, 87-93`) are fully plumbed through the error converter but never constructed by any kernel. These should NOT be deleted: they are the missing pieces for the IntegralDivide overflow and Byte/Short error-class bugs filed separately. Listed here so the cleanup does not remove them by mistake.

### Describe the potential solution

Remove the two dead `CometEvalMode` helpers, resolve the `allow_incompat` ambiguity (remove or document), and leave the two error variants in place for the correctness fixes.

### Additional context

Found by an ANSI-mode audit of all native expressions against Spark 4.1.1.

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.