sunchao commented on PR #5302: URL: https://github.com/apache/datafusion-comet/pull/5302#issuecomment-5852679985
Found one **P2 correctness regression** at [CometCast.scala:213](https://github.com/apache/datafusion-comet/blob/9dd2e7f67e7ae2f1218f8b2c7103624376447745/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala#L213): **the new dispatcher route enables incorrect downstream sorting.** Previously, a complex cast containing `Collate` left the projection and following local sort in Spark. The new guard dispatches the entire cast, allowing both operators into Comet. Multi-key native sorting skips collation checks and compares string bytes. Reproduced with a single-partition Parquet table containing `(s, a, id) = ('a', 1, 0), ('B', 1, 1)`: ```sql SELECT CAST( struct(a AS a, s COLLATE utf8_lcase AS s) AS STRUCT<a: STRING, s: STRING COLLATE UTF8_LCASE> ) AS st, id FROM t SORT BY st, id; ``` Spark returns **`a, B`**; Comet returns **`B, a`**. Removing only the new guard restores the correct result. This newly exposes the existing bug tracked in [#6158](https://github.com/apache/datafusion-comet/issues/6158). Fix that first, include recursive sort guards here, or preserve Spark fallback for the affected casts. Validation: - Five independent review passes; no other actionable findings. - Confirmed the planner change on exact head `9dd2e7f` versus base `1c25b492`. - Runtime tests used CI merge `279dbf7` with its matching native library: all **21 PR tests passed**, the regression probe failed, and the guard-off control passed. - [Current CI](https://github.com/apache/datafusion-comet/actions/runs/36198848992) has 24 successful and 14 skipped checks. I did not run the full Spark-version matrix locally. I recommend addressing this before merge. Nothing posted to GitHub. -- 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]
