viirya edited a comment on issue #24557: [SPARK-27653][SQL] Add max_by() SQL aggregate function URL: https://github.com/apache/spark/pull/24557#issuecomment-490746455 @JoshRosen Thanks for the review! > * Could you also implement min_by(x, y)? Yes. I originally planed to have separate PR for it. I'm fine to add it here. A shared abstract superclass to share code is good. > * Presto also has three-argument versions of max_by / min_by: Agreed. We don't need three-argument versions now. If we need it, we can add it in a followup. > * Were there any bugs in older implementations of Presto version that we might have replicated here? Or Presto tests for edge-cases that we could emulate? * For using rows / structs as the ordering value, I also think it would work. I will add few tests. * For null ordering values, I already have few test cases. I checked Presto's results and they are matched. Let me see double-check if we've covered the same edge-case.
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
