fix: correct two nightly test failures on Spark 3.4 and 4.2 - #6156
Merged
Merged
Conversation
Before Spark 3.5 (SPARK-44236), setting spark.sql.codegen.factoryMode to NO_CODEGEN did not turn whole-stage codegen off. Spark 3.4 therefore keeps an ungrouped decimal SUM unbounded and recovers from an intermediate overflow, the same as the native accumulator. Only apply the factory-mode condition of the max-precision SUM fallback on 3.5+, and expect the query to run natively with the recovered sum on 3.4. captureWritePlan registered its QueryExecutionListener while an earlier write's event could still be queued on the async listener bus. That event was then captured in place of the write under test. Drain the bus before registering and after the write, and hold the plan in an AtomicReference.
andygrove
added
the
run-all-spark-profiles
sunchao
approved these changes
Sep 23, 2026
Member
Author
|
Not sure why required checks is not green |
andygrove
enabled auto-merge
September 23, 2026 17:09
auto-merge was automatically disabled
September 23, 2026 17:42
Pull request was closed
rich7420
added a commit
to rich7420/datafusion-comet
that referenced
this pull request
Sep 24, 2026
Resolve the Parquet write-plan capture conflict by retaining main's implementation from apache#6156, including its listener-bus drains and AtomicReference. The original fix is already covered upstream.
7 of 72 tasks
andygrove
added a commit
that referenced
this pull request
Sep 28, 2026
Backport of #6108 to branch-1.0. #6108 merged to main as an empty commit (df8e153) because #6156 had already made the same change to the helper, so this ports the captureWritePlan hunk of dd68a53. captureWritePlan registered its QueryExecutionListener while an earlier write's end event could still be queued on the asynchronous listener bus, and returned the first plan it saw. A test that seeds data with Comet disabled could then assert on the seed write's plan. The helper now drains the bus before registering and after the write. On branch-1.0 the helper lives in CometParquetWriterSuite, not in CometParquetWriterTestBase. The added and removed lines match dd68a53.
This was referenced Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 isspark.sql.codegen.factoryMode=NO_CODEGEN. That setting only disables whole-stage codegen from Spark 3.5 on (SPARK-44236). Spark 3.4'sCollapseCodegenStageschecks onlywholeStageEnabled. 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 earlierINSERT INTO comet_write_source VALUES ..., which runs with Comet disabled, not the insert under test.captureWritePlanregistered itsQueryExecutionListenerwhile 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.getSupportLevelapplies theNO_CODEGENfactory-mode condition only on Spark 3.5+. On 3.4 that query now stays native, which matches Spark's codegen result.NO_CODEGENcase inCometAggregateSuiteexpects the fallback on 3.5+. On 3.4 it expects a native plan and the recovered sum.CometParquetWriterTestBase.captureWritePlandrains the listener bus withCometListenerBusUtils.waitUntilEmptybefore registering its listener and after the write. It holds the plan in anAtomicReferenceand no longer polls for up to 15s. This follows the helper inCometIcebergWriteDetectionSuite.NO_CODEGENfallback applies on Spark 3.5+.How are these changes tested?
These are changes to existing tests. Run locally:
CometParquetWriterSuiteand thedecimal sumtests inCometAggregateSuitepass (61 tests).-Pspark-3.4): thedecimal sumtests inCometAggregateSuitepass (12 tests), including the test that failed in the nightly.CometParquetWriterSuitealso passes (36 tests), which covers the 3.x use of the changed helper.The
run-all-spark-profileslabel runs the Comet suites on 3.4, 3.5, 4.0 and 4.2 in CI.