chore: Fallback to Spark for windows functions #2726
Conversation
| | `spark.comet.exec.sortMergeJoinWithJoinFilter.enabled` | Experimental support for Sort Merge Join with filter | false | | ||
| | `spark.comet.exec.takeOrderedAndProject.enabled` | Whether to enable takeOrderedAndProject by default. | true | | ||
| | `spark.comet.exec.union.enabled` | Whether to enable union by default. | true | | ||
| | `spark.comet.exec.window.enabled` | Whether to enable window by default. | false | |
There was a problem hiding this comment.
this was the only changes, looks like IDE reformatted the file
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2726 +/- ##
=============================================
- Coverage 56.12% 22.97% -33.15%
+ Complexity 976 441 -535
=============================================
Files 119 147 +28
Lines 11743 13817 +2074
Branches 2251 2369 +118
=============================================
- Hits 6591 3175 -3416
- Misses 4012 10087 +6075
+ Partials 1140 555 -585 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| createExecEnabledConfig("expand", defaultValue = true) | ||
| val COMET_EXEC_WINDOW_ENABLED: ConfigEntry[Boolean] = | ||
| createExecEnabledConfig("window", defaultValue = true) | ||
| createExecEnabledConfig("window", defaultValue = false) |
There was a problem hiding this comment.
I think we need to double check how Comet works with exec.*.enabled params.
In this example I disabled windows and actually expected that Comet would fallback silently to Spark for windows, however I got a bunch of weirdest issues, including issues on native side
2025-11-08T05:18:43.0134899Z [info] org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 3931.0 failed 1 times, most recent failure: Lost task 0.0 in stage 3931.0 (TID 4375) (10306839d6c4 executor driver): org.apache.comet.CometNativeException: assertion failed: !self.format.is_null()
There was a problem hiding this comment.
yes, this section of code is very problematic and brittle. I am trying to understand this too.
| } | ||
| } | ||
|
|
||
| test("lead/lag should return the default value if the offset row does not exist") { |
There was a problem hiding this comment.
It looks like the same test is here as well as in CometWindowExecSuite ?
There was a problem hiding this comment.
Moved windows related tests from all files to CometWindowExecSuite
3 participants
Which issue does this PR close?
Related to #2721
Closes #.
Rationale for this change
Currently windows functions are enabled by default, however we can spot numerous correctness issues which need to be addressed separately. List of correctness issues can be found in #2721
What changes are included in this PR?
Disable window functions until correctness issues addressed.
Move some other window functions tests to
CometWindowExecSuiteHow are these changes tested?