peter-toth commented on PR #58421: URL: https://github.com/apache/spark/pull/58421#issuecomment-5495792849
Thanks @dongjoon-hyun. All three taken, in https://github.com/apache/spark/pull/58421/commits/f2f417e77e6aa6f9da7886eac9a75b5fd91df0ae. 1. The description was stale and that one is on me. I refreshed the commit message when I rebased onto #58335 and forgot the description. Both snippets now read `Some(KeyReducer(reducer: Reducer[Any, Any], _))`, and so does the sentence about the erased type test. 2. Restored, as one `assert` before the key loop instead of one per key. I asserted `reducerArray.length` against the same bound there too, since that is the length mismatch you named in the LGTM list, and the comment records that neither is reachable from today's two call sites. 3. Reworded to say what the hoist buys. The scale argument stays, moved next to the loop that pays it. On the reachability: agreed, and thanks for tracing both call sites rather than taking the equality on trust. The `assert` felt like the right weight for it, so a third caller finds out at the boundary rather than through an `ArrayIndexOutOfBoundsException` or a silently short key. The description's suite total moved from 286 to 305 with the rebase, since `KeyGroupedPartitioningSuite` grew in #58335. Re-ran the same eight suites on the new head. -- 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]
