zhangfengcdt commented on PR #6928: URL: https://github.com/apache/datafusion-comet/pull/6928#issuecomment-6321660354
> Thanks for modeling the merge stages. The `count(DISTINCT)` case now matches Spark, and having it in `percentile_zero_tie.sql` as well as the unit test is exactly what I was hoping for. Keeping `partial_ends` and `partial_depths` empty outside `FLOAT`/`DOUBLE` on 3.5.2+ answers comphead's concern about the extra state, and documenting the sort-based fallback and early emit makes sense given Spark's own answer moves there. > > Could you make the `zeroOrderNote` wording name Comet's spill explicitly? You mentioned that a spill inside a merge stage nests one level deeper than Spark, which is a case where Spark's answer is fixed and only Comet's can move. As written, "nor its own aggregates emitting or spilling" reads as Spark's aggregates, and this text ends up in the user-facing compatibility page. > > Since the behavior depends on the Spark version and the replay mirrors Spark's `OpenHashMap` hashing, I've added `run-all-spark-profiles` so the fixtures also run on 3.4, 3.5, 4.0 and 4.2. Please hold the merge until that run is green. Sure, the note now changed to match Comet's own early emit and spill, including the merge-stage spill that nests one level deeper than Spark. -- 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]
