Eric5553 edited a comment on issue #27368: [SPARK-30651][SQL] Add detailed information for Aggregate operators in EXPLAIN FORMATTED URL: https://github.com/apache/spark/pull/27368#issuecomment-579046506 > @Eric5553 Since the implementation is same for variations of aggregate operator, i was wondering if it makes sense to have a base class where we put these common code ? what do you think ? @dilipbiswal Thanks so much for review! Yeah, this is a concern when I implement for the three aggregate operators. The groupingExpressions(shown as 'Keys') and aggregateExpressions(shown as 'Functions') are defined in each aggregate operator but not in common super class. So I think we cannot abstract the `verboseStringWithOperatorId` logic here until we abstract these aggregate attributes. I think the visitor pattern proposed in the discussion of your initial PR would provide more flexibility. By then, we could separate input/output as a common rule for example. I can give a try if you got any suggestion on this concern, thanks!
---------------------------------------------------------------- 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]
