Add a fallback-invariance suite: query outcome should not change when a compatible expression is forced from native to Spark

[4ktLuffy]

What is the problem the feature request solves?

QueryPlanSerde honours spark.comet.expression.<Name>.enabled=false as forced fallback, and there are 293 expressions registered to it:

grep -oE "classOf\[[A-Za-z0-9_]+\] ->" spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala | sort -u | wc -l

That gives a free invariant, with no product change needed: for any expression Comet rates compatible, a query's outcome — its rows, or the error it raises — must be identical whether the expression is evaluated natively or forced back to Spark.

Four distinct tests exercise that lever today, each pinning one hand-picked expression: disable expression using dynamic config (Add) in CometExpressionSuite, plus the ToPrettyString, WidthBucket and StringDecode suites. All four do check the answer, via checkSparkAnswerAndOperator on the native leg and checkSparkAnswerAndFallbackReason on the forced leg. What is missing is a mechanical sweep across the registry, and any systematic check that the two legs agree on which error they raise.

The evidence, and its limits

To test whether an invariance sweep of this shape has teeth, I reapplied the pre-fix condition from #5218 — dropping the inputOrdinals.size == 1 || rootChildrenAreLeaves(expr) narrowing from CometBatchKernelCodegen.canShortCircuitNulls, so the null short-circuit again applies to a whole tree — and ran a set of detectors against that one probe on one build:

Detector	Result
CometFuzzMathSuite	30/30 pass — miss
CometCodegenFuzzSuite	28/28 pass — miss
CometCodegenHOFSuite	5/5 pass — miss
org.apache.comet.rules.CometExecRuleSuite	29/29 pass — miss
CometCodegenSuite	1 failure — the (#5218) regression guard
CometCodegenSourceSuite	2 failures — both (#5218) guards
proposed suite	fails: AddMonths default=VALUE(2024-02-01) forced=THREW(CAST_INVALID_INPUT)

Two honest caveats on that table, stated up front. The three suites that catch the probe are #5218's own regression tests, shipped with its fix — their failure shows the probe is genuinely behaviour-changing, and nothing more. And my corpus contains AddMonths because #5218 named it, so the last row is a validation replay, not an independent rediscovery: CometCodegenSuite's guard already covers that same query end-to-end. What the table does support is narrower — four generic mechanical detectors miss this class, including a fuzzer aimed squarely at codegen, while a fallback-invariance comparison surfaces it with a named witness. The comparator is not specific to add_months; it would flag this shape for any expression in the corpus.

The oracle also differs from the existing guard's. CometCodegenSuite's test compares Comet against Spark directly. This suite compares Comet-native against Comet-with-that-expression-forced-to-Spark, which is the config lever no current test sweeps.

Same build with the probe reverted: 24 pass / 0 fail / 1 skipped-vacuous — the single delta is the witness above.

Describe the solution

A deterministic scalatest suite, no product code, over the existing CometFuzzTestBase fixture. Per expression it runs one fixed query twice — default config, then .enabled=false — and:

• gates every comparison on evidence the flip actually moved execution. The default leg must show Comet operators and no Spark ProjectExec; the forced leg must show a Spark ProjectExec. A plan that shows neither proves nothing and is reported SKIPPED-VACUOUS, never counted as a pass.
• compares three-valued outcomes (Rows vs Threw(errorClass)), so a value on one leg against an exception on the other is a failure with a witness rather than a harness error. Differing error classes are also a failure.
• treats a divergence on an incompatible-rated expression as EXCUSED — logged, never a pass.
• canonicalises NaN, -0.0 and NULL to distinct tokens, and row order by sorting.

25 expressions × the base suite's 3 shuffle/C2R variants = 75 comparisons in ~10 s, so it is per-PR viable. Compiles and passes under spark-3.4, spark-3.5 and spark-4.1.

Current scope is deliberately 25 of 293 expressions. Widening is mechanical, and the bind gate is what keeps the coverage claim honest as it grows — without it a sweep like this silently overstates itself, which happened three times while this one was being built.

Related work I checked first

• Discussion: remove incompatible native implementations that have a codegen dispatch fallback? #4654 (dispatcher vs incompatible native impls) is the reason the sweep reports one skip rather than a pass: StringTranslate is incompatible-rated, so by default it never runs natively and there is no native leg to compare against. Whether an expression is reachable natively at all is a dispatcher question, and the bind gate makes that visible instead of silently counting it as verified.
• Partial project fallback: keep a projection/filter native by evaluating an unsupported subexpression in the JVM #4825 / feat: partial project fallback (JVM expression detour) #4827 (partial project fallback, JVM expression detour) would change the plan shape this suite reads: it keeps an operator native by routing an unsupported subtree through the JVM, so a forced-fallback leg might no longer surface as a Spark ProjectExec. That work is default-off today, so there is no conflict now — but the bind gate is the part that would need updating alongside it, and it would report SKIPPED-VACUOUS rather than passing silently if the shape changed underneath it.
• I did not find an existing invariance sweep over this lever; if one exists and I missed it, that is the fastest way to close this issue.

Additional context

This came out of a systematic, AI-assisted audit of Comet's verification machinery, human-verified at each step and mutation-tested with positive controls. Given #4085's note that LLM-assisted audits have outperformed fuzzing here, the suite is built to that lesson: fast, deterministic, named witnesses. Happy to share the full evidence trail, including the negative results — the audit also produced two candidate findings about the config lever that dissolved on verification, and those are not being filed.

PR attached.

[4ktLuffy]

To sharpen the distinction above, since it is the obvious first question: what this suite adds is population coverage, not a new instance.

CometCodegenSuite's (#5218) test guards that instance — add_months over ('notadate', NULL) with ANSI on — and it guards it well. But the mechanism it protects against is general: canShortCircuitNulls gates on isNullIntolerant(expr) && allNullIntolerant(expr) plus the ordinal count, so the same short-circuit reasoning applies to any multi-input NullIntolerant tree. CometCodegenSourceSuite's two guards already demonstrate that generality by using different expressions for the same shape (Add(Cast(..), ..) and MakeTimestamp(Cast(..), ..)).

What is missing is mechanical: nothing sweeps error-class parity under forced fallback for the other 292 registered expressions. Each existing guard was written because someone already knew where to look. The invariant here does not need to know — it compares outcomes for whatever is in the corpus, and reports a named witness when they diverge.

So the honest accounting is: the replay validated the harness, #5218's own tests own that instance, and the contribution is the sweep that would cover the rest of the population as the corpus grows. If maintainers would rather see the corpus widened substantially before this is worth merging, I am happy to do that in this PR — widening is mechanical, and the bind gate is what keeps the coverage number honest as it does.