test: cover round expression routing configurations

[rich7420]

Which issue does this PR close?

Part of #4616.

Rationale for this change

The round SQL tests do not check routing with the codegen dispatcher disabled or when allowIncompatible changes.

What changes are included in this PR?

Run the fixture with allowIncompatible both disabled and enabled. Assert that disabling the dispatcher keeps integral and decimal inputs native and sends float and double inputs to Spark with the expected fallback reason.

How are these changes tested?

Both configurations pass locally on Spark 4.1. Injected regressions that ignore the dispatcher-disable guard or make native support depend on the dispatcher are caught by the new assertions.

Fork CI passed, including all six expressions jobs across Spark 3.4–4.2 and macOS Spark 4.0.

[andygrove]

Thanks for picking this up. The new expect_native query with the dispatcher off is a real regression guard, and the fallback reason assertions are correct. I have two concerns about how the coverage is structured, in the inline comments.

One more thing on the PR description. It says "Enabling allowIncompatible must not change either route." Since round has no Incompatible case, it might be worth rewording that so it doesn't suggest to the next reader that the flag plays a role in round routing.

[andygrove]

I don't think this matrix can observe anything. CometRound.getSupportLevel only returns Unsupported or Compatible, and the Unsupported arm in exprToProtoInternal never reads allowIncompatible. So both matrix values run the exact same code, and the whole file (including the overflow tables and every existing query) runs twice. The sql-file-tests doc asks us not to add ConfigMatrix speculatively.

Could we drop the matrix here? If the goal is to guard against float/double being reclassified as Incompatible in the future, that would be better handled by whichever PR makes that change adding the row 2 coverage at that point.

[andygrove]

This is the first fixture in sql-tests/ that changes config with a mid-file SET. It works because the Config: ...codegen.enabled=true line at the top makes withSQLConf restore the flag after the file, but nothing in the fixture says that, and the top-of-file directive now reads as if the dispatcher is on for the whole file. If someone later removes that seemingly redundant Config line, the SET leaks and every fixture that runs afterwards silently loses dispatcher coverage.

Could we follow the existing pattern instead and move these three queries into their own fixture with -- Config: spark.comet.exec.scalaUDF.codegen.enabled=false at the top? misc/codegen_dispatch_disabled_fallback.sql is the precedent. That keeps each file under a single config.

[sunchao]

Correctness

Reviewed f79d7b21 against base 3810936b. The existing fixture checked native integral/decimal rounding and dispatched floating-point rounding, but did not pin the dispatcher-disabled behavior. This change adds three queries that require integral and decimal expressions to remain native and float/double expressions to fall back with the disabled-dispatcher reason. It changes one SQL test file and no production code.

I checked RoundBase and its tests on the maintained Spark 3.5 and 4.0 branches. Both use HALF_UP rounding, preserve nulls and special floating-point values, and distinguish ANSI overflow from legacy wrapping. The fixture runs with ANSI disabled and constant folding excluded. Its existing column and literal cases retain coverage of nulls, NaN/infinities, signed zero, decimal-string rounding boundaries and legacy long overflow. This PR does not establish new ANSI coverage. The new assertions compare Spark results as well as checking native routing or the specific fallback reason. I found no new or remaining verified P1/P2 issue.

Validation

At 2026-09-12 21:37:04 UTC, CI had 41 successful and 11 skipped checks, including successful Required Checks. I verified both matrix rows in the Spark 3.5 expressions job and Spark 4.0 expressions job. Their checkout 596fc7ce has parents exactly this base and head. Its only change from the base is the reviewed fixture, with the same blob as the head. The relevant runner and routing code are unchanged across the head, merge base and current base. I did not run separate local JVM/native tests or reproduce the author's injected regressions. Maintained Spark 3.4/4.1 source coverage remains unavailable.

Performance

There is no production execution change. The matrix runs all 14 queries and seven statements twice even though CometRound returns only Compatible or Unsupported, neither of which reads this allowIncompatible flag. The second row took 1.191 seconds in the inspected Spark 3.5 job and 1.310 seconds in Spark 4.0. Those are test durations, not expression benchmarks. The existing matrix discussion already identifies this redundant work. I have no additional performance finding.

Design

The supported-type query directly guards against making native support depend on the dispatcher. Separate float and double queries make the expected fallback reason unambiguous. The mid-file SET currently stays within withSQLConf: the top-level dispatcher directive saves and restores that same key, including on failure, and each matrix case gets a fresh scope. I found no current configuration leak. The existing fixture-structure discussion already covers making this dependency clearer through a separate disabled-dispatch fixture.

Abstraction & complexity

The change uses the existing SQL fixture parser and routing assertions without adding helpers or production abstractions. expect_native(round) checks both the native expression set and absence from the dispatched set, while expect_fallback checks results and the named reason. This is an appropriate level for these routing checks. I have no additional findings beyond the existing discussion.