rich7420 opened a new pull request, #5718:
URL: https://github.com/apache/datafusion-comet/pull/5718

   ## Which issue does this PR close?
   
   Closes #5624.
   
   ## Rationale for this change
   
   `CometColumnarToRowBenchmark` overrides `getSparkSession` without setting 
`spark.shuffle.manager`. With Comet shuffle enabled by default, this disables 
the extension, so all three arms measure Spark's `ColumnarToRowExec` despite 
their labels. The fixed-width and primitive fixtures also contain small 
integers that trigger the scan safety check.
   
   ## What changes are included in this PR?
   
   All changes are in `CometColumnarToRowBenchmark.scala`.
   
   - Set `CometShuffleManager` before creating the Spark context.
   - Disable the unsigned-small-integer safety check for these Spark-written 
fixtures, which contain no unsigned small integers.
   - Run each arm once before timing and inspect the actual `.noop()` write 
plan through a query listener. Require the expected Spark, Comet JVM, or Comet 
native conversion, and write successful verification to the benchmark output.
   
   The check fails before timing if an arm falls back or loses its conversion. 
Listener synchronization and plan checks are outside the timed cases.
   
   ## How are these changes tested?
   
   Benchmarks are compiled and linted in CI; execution was checked locally with 
a temporary driver calling all 10 existing fixture methods with 16,384 rows 
each on Spark 4.1.3 / Scala 2.13.17 / Java 17. All 30 execution-path checks and 
benchmark cases completed.
   
   Both fallback paths were checked separately: removing the shuffle-manager 
setting from a temporary compiled copy, and re-enabling the small-integer 
safety check for the fixed-width fixture. Each was rejected at the Comet JVM 
preflight with Spark's `ColumnarToRowExec` in the captured write plan.
   
   [The fork CI 
run](https://github.com/rich7420/datafusion-comet/actions/runs/33977397812) 
passed preflight, Rust formatting/clippy, benchmark compilation, and Spark 4.0 
Scala compilation/lint. Local Spotless, RAT, and whitespace checks also passed.
   
   Local execution used a native debug build, so the timings are not 
performance evidence. Local JVM compilation required 
`-DcompileOrder=JavaThenScala` to work around a name-hash error in unchanged 
production code. The full Spark-version test matrix was not run.
   


-- 
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]

Reply via email to