4ktLuffy opened a new pull request, #5329: URL: https://github.com/apache/datafusion-comet/pull/5329
## Which issue does this PR close? Closes #5328. ## Rationale for this change For every expression Comet rates compatible, forcing it back to Spark with `spark.comet.expression.<Name>.enabled=false` must not change a query's outcome — neither its rows nor whether and what it throws. The invariant is free: the config already ships, and `QueryPlanSerde` already honours it for all 293 registered expressions. It is not currently swept. Four tests exercise the lever, each on one hand-picked expression; all four check the answer, but none checks a range of expressions mechanically and none checks that the two legs raise the *same* error. Replaying #5218's pre-fix logic shows the shape has teeth: the defect is invisible to `CometFuzzMathSuite` (30/30), `CometCodegenFuzzSuite` (28/28), `CometCodegenHOFSuite` (5/5) and `CometExecRuleSuite` (29/29), while this suite names it: ``` FAIL-OUTCOME [errors] AddMonths default=VALUE(2024-02-01) forced=THREW(CAST_INVALID_INPUT) ``` To be clear about what that is and isn't: `AddMonths` is in the corpus *because* #5218 named it, so this is a validation replay rather than an independent find, and `CometCodegenSuite` already guards that exact query end-to-end. The generic-detector misses are the load-bearing part, and the comparator is not specific to `add_months` — it flags this shape for any corpus expression. ## What changes are included in this PR? One new suite, `CometFallbackInvarianceSuite`, extending `CometFuzzTestBase`. No product code. - **Fixed query corpus** — 25 expressions over the existing seeded fuzz fixture, no new fixtures. A value-parity section with ANSI off, and an error-parity section with ANSI set explicitly on so it behaves identically on every Spark profile. - **Bind gate, mandatory.** Before any comparison, the executed plan must prove the flip moved execution: default leg shows Comet operators and zero Spark `ProjectExec`; forced leg shows a Spark `ProjectExec`. A plan showing neither is `SKIPPED-VACUOUS` and reported — never a pass. Plan presence is captured whether or not `collect()` succeeded, so a leg that throws is gated too. - **Three-valued outcome comparison** (`Rows` vs `Threw(errorClass)`): value-vs-throw is a failure, differing error classes are a failure, row-count mismatch is a failure. Error parity is where #5218 lives, so these comparator rules are load-bearing rather than incidental. - **Verdicts** — compatible-rated divergence `FAIL` · value-vs-throw `FAIL` · differing error class `FAIL` · length mismatch `FAIL` · incompatible-rated divergence `EXCUSED`, logged but never certified · NaN / `-0.0` / NULL as distinct tokens · row order canonicalised by sorting. ## How are these changes tested? The suite is the test. On a clean tree: ``` FALLBACK-INVARIANCE-RESULT pass=24 fail=0 excused=0 vacuous=1 FALLBACK-INVARIANCE SKIPPED-VACUOUS [values] StringTranslate not-native-by-default ``` 24 pass, 0 fail, 1 skipped-vacuous per variant × 3 variants, ~10 s. The one skip is honest and expected: `StringTranslate` is incompatible-rated, so it already runs on Spark by default and there is no native leg to compare against. Verified under `spark-3.4`, `spark-3.5` and `spark-4.1`. Validated by injected divergence twice: a deliberately broken `spark_decimal_div` as a positive control (also caught by `CometFuzzMathSuite`, as it should be), and the #5218 pre-fix replay. The probe run reports `pass=23 fail=1` — one delta from clean, and it is the witness quoted above. **On the bind gate.** It earned its place three times over while this suite was being written. An early version reported 22 passes where the truth was 19 passes and 3 vacuous. A second pinned ANSI off and mapped throw-vs-value to a harness error, making it structurally blind to the very bug class it exists for. A third used `ORDER BY` for determinism, which introduces a shuffle and roots the executed plan at `AdaptiveSparkPlanExec` — whose `children` is empty, so plan inspection silently read zero for every query in that section and the gate stopped working entirely. Comet's own `spark.comet.expression.*` tests sidestep all of this by using simple queries with no shuffle; this suite now does the same, avoiding `ORDER BY` and canonicalising row order in the comparator instead. **Scope, stated plainly.** 25 of 293 expressions. The suite owns one historical bug (#5218) and is not claimed to cover operator-level mixed execution, fallback-*decision* logic, or scan-metadata mismatches — #4813, #4051, #4789 and #2720 are different mechanisms and this invariant would not have caught any of them. Provenance note: produced during an AI-assisted audit of Comet's verification machinery, human-verified at each gate; mutation experiments were run with positive controls and the tree restored clean afterward. -- 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]
