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

   ## Which issue does this PR close?
   
   Closes #6214.
   
   ## Rationale for this change
   
   The 2026-09-25 nightly failed the Spark 4.0 `sql_core-1` job on one test, 
`CollationSuite` "hash agg is not used for non binary collations", with `None 
was empty (CollationSuite.scala:467)`. That is the assertion that the 
`UTF8_BINARY` `GROUP BY` plan contains a Spark `HashAggregateExec`.
   
   The test had been passing by accident. Spark 4.0's `DatasourceV2SQLBase` 
runs `spark.sessionState.conf.clear()` after every test, which removes 
`spark.shuffle.manager` from the session conf. Until #6195, 
`isCometShuffleManagerEnabled` read that setting from the session conf, so 
Comet shuffle was off for every test after the first one in the suite. This 
query's final aggregate stayed on Spark, above a Spark exchange. #6195 checks 
the shuffle manager that `SparkEnv` holds, which is `CometShuffleManager`. 
Comet shuffle now plans the exchange, and the final aggregate becomes a 
`CometHashAggregateExec`.
   
   That is what the test has always seen on Spark 4.1, where 
`DatasourceV2SQLBase` only unsets two catalog configs, and the 4.1.3 diff 
already accepts `CometHashAggregateExec` in this assertion. #6195 ran only the 
Spark 4.1 suite, and the 4.0 suite runs nightly, so the nightly was the first 
run that could catch this. The new behaviour is the correct one, so the fix 
belongs in the diff.
   
   The same nightly's Iceberg 1.9 failure is unrelated. It is a known upstream 
flake that this PR does not touch; details are on #6214.
   
   ## What changes are included in this PR?
   
   - `dev/diffs/4.0.4.diff` is regenerated from a v4.0.4 clone with the 
existing diff applied. It gains the same two lines the 4.1.3 diff has: the 
`CometHashAggregateExec` import and a `case _: CometHashAggregateExec => ()` in 
the `UTF8_BINARY` assertion.
   - Regenerating also updates the `index` line for 
`ParquetRowIndexSuite.scala`. #6046 removed a hunk from that file by hand and 
left the old post-image hash behind. The file's content is unchanged. Before 
making the edit, I checked that regenerating the unmodified diff reproduced the 
committed file except for that one line.
   
   ## How are these changes tested?
   
   The regenerated diff applies cleanly to a fresh v4.0.4 checkout, and the 
resulting `CollationSuite.scala` matches the edited clone. Spark's test sources 
can't be built locally here, so this PR carries `run-spark-4.0-tests`, which 
compiles the patched Spark and runs `CollationSuite` in `sql_core-1`.
   


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