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

   ## Which issue does this PR close?
   
   Closes #6134.
   
   ## Rationale for this change
   
   The 2026-09-23 nightly failed two Comet suite jobs, each on one test. 
Neither failure is a correctness bug in Comet.
   
   **Spark 3.4 `[exec]`: `decimal sum without codegen falls back at maximum 
precision`.** #6041 falls back to Spark for an ungrouped decimal SUM at 
precision 38 whenever Spark would run the aggregate without codegen. One of its 
conditions is `spark.sql.codegen.factoryMode=NO_CODEGEN`. That setting only 
disables whole-stage codegen from Spark 3.5 on 
([SPARK-44236](https://issues.apache.org/jira/browse/SPARK-44236)). Spark 3.4's 
`CollapseCodegenStages` checks only `wholeStageEnabled`. On 3.4, Spark 
therefore keeps the ungrouped sum unbounded and recovers from the intermediate 
overflow under ANSI. Comet fell back anyway, so both engines returned the 
recovered value and the test's expected error never came (`got Spark None and 
Comet None`). The PR tier only runs Spark 4.1, so this did not show up before 
merge.
   
   **Spark 4.2 `[scans]`: `INSERT INTO ... SELECT is visible to subsequent 
reads`.** The failure message shows that the captured plan belonged to the 
test's earlier `INSERT INTO comet_write_source VALUES ...`, which runs with 
Comet disabled, not the insert under test. `captureWritePlan` registered its 
`QueryExecutionListener` while that earlier event could still be queued on the 
async listener bus, so the listener caught the stale event. The test is flaky; 
nothing here is specific to Spark 4.2.
   
   ## What changes are included in this PR?
   
   - `CometHashAggregateExec.getSupportLevel` applies the `NO_CODEGEN` 
factory-mode condition only on Spark 3.5+. On 3.4 that query now stays native, 
which matches Spark's codegen result.
   - The `NO_CODEGEN` case in `CometAggregateSuite` expects the fallback on 
3.5+. On 3.4 it expects a native plan and the recovered sum.
   - `CometParquetWriterTestBase.captureWritePlan` drains the listener bus with 
`CometListenerBusUtils.waitUntilEmpty` before registering its listener and 
after the write. It holds the plan in an `AtomicReference` and no longer polls 
for up to 15s. This follows the helper in `CometIcebergWriteDetectionSuite`.
   - The compatibility guide notes that the `NO_CODEGEN` fallback applies on 
Spark 3.5+.
   
   ## How are these changes tested?
   
   These are changes to existing tests. Run locally:
   
   - Spark 4.1 (default profile): `CometParquetWriterSuite` and the `decimal 
sum` tests in `CometAggregateSuite` pass (61 tests).
   - Spark 3.4 (`-Pspark-3.4`): the `decimal sum` tests in 
`CometAggregateSuite` pass (12 tests), including the test that failed in the 
nightly. `CometParquetWriterSuite` also passes (36 tests), which covers the 3.x 
use of the changed helper.
   
   The `run-all-spark-profiles` label runs the Comet suites on 3.4, 3.5, 4.0 
and 4.2 in CI.
   


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