LinSimon-901101 opened a new pull request, #6206:
URL: https://github.com/apache/datafusion-comet/pull/6206

   ## Which issue does this PR close?
   
   Closes #6158.
   
   ## Rationale for this change
   
   Multi-column sort keys bypassed the collation check in `supportedSortType`, 
allowing Comet to compare non-default collated strings by raw bytes. A 
`row_number()` window over `UTF8_LCASE` and an integer tie-breaker consequently 
assigned different row numbers from Spark when a single-partition exchange 
bypassed the existing shuffle guards.
   
   ## What changes are included in this PR?
   
   - Check every sort key with the existing recursive 
`hasNonDefaultStringCollation` helper before applying the unchanged 
single-column sorting restrictions. Unsupported collations now fall back to 
Spark with an explicit reason, including in TopK plans.
   - Add a shared Spark 4.x regression suite covering the reported window 
query, secondary and nested keys, round-robin local sorting, TopK, and 
default-collation controls. Register the suite in both CI workflows.
   - Add an adversarial uppercase value to the existing WindowGroupLimit 
collation fixture and update its comments to account for sort fallback 
happening first.
   
   ## How are these changes tested?
   
   The original window regression was run before the fix and failed with the 
exact incorrect row numbers from #6158. Its physical plan contained 
`CometWindowExec` over `CometSort` over a `SinglePartition` columnar exchange. 
The same test passes after the fix and asserts Spark sort fallback.
   
   Verified locally:
   
   - Spark 4.2.0: 13 tests passed across `CometSortCollationSuite`, the 
`window_group_limit_collation` SQL fixture, and existing `CometExecSuite` tests 
matching `Sort on`.
   - Spark 4.1.3: 34 tests passed across `CometSortCollationSuite`, 
`CometCollationSuite`, the `window_group_limit_collation` SQL fixture, and 
existing `CometExecSuite` tests matching `Sort on`.
   - Spark 4.0.4: the same selection passed all 39 tests, together with 
Scalafix in CHECK mode using the semanticdb profile.
   - Spark 3.5.9 (JDK 17): all 26 existing tests passed from `CometExecSuite` 
matching `Sort on` and `CometExpressionSuite` matching `sort`.
   - Spark 3.4.3 (JDK 17): the same existing-test selection as Spark 3.5 passed 
all 26 tests.
   - Scalastyle, Spotless, CI suite registration, and `git diff --check` passed.
   
   The TopK test evaluates `COLLATE` below an explicit single-partition 
exchange, so an unrelated projection fallback cannot satisfy the expected sort 
fallback reason. Default-collation controls explicitly assert native Sort and 
TopK operators remain in the plan.
   


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