sunchao commented on PR #5421: URL: https://github.com/apache/datafusion-comet/pull/5421#issuecomment-5430479215
Thanks @andygrove . You’re right that the AVG examples alone don’t demonstrate what #5421 adds after #5420’s native fixes. I’ve now reproduced a separate failure. I kept #5420’s native library and removed only the call to `revertUnsafePartialAggregates`. On Spark 4.1.3 / JDK 17, `COLLECT_LIST` and `COLLECT_SET` crash in Spark’s `Collect.deserialize` when grouping by a stored `DECIMAL(19,0)` key, with Comet shuffle enabled, mode native, and native hash partitioning enabled. This happens with AQE both off and on. Restoring the guard makes all four cases match Spark. The decimal key makes the native shuffle fall back, leaving a Comet partial feeding a Spark final. That partial produces an array buffer, while Spark expects serialized binary state. The shuffle-disabled sweep doesn’t exercise this path because it prevents the ObjectHashAggregate partial from converting in the first place. The existing AVG regression passes with or without the guard. So there is an independent correctness case for the planner repair. That still doesn’t justify every exclusion in the current compatibility flag. I agree we should distinguish the two directions and preserve native partials where their buffers are compatible. The duplication is because #5420 currently includes #5421 as a prerequisite. I’ll clarify that relationship and update #5421’s tests and description around this separate failure. -- 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]
