akpatnam25 commented on a change in pull request #33644:
URL: https://github.com/apache/spark/pull/33644#discussion_r690888342



##########
File path: core/src/main/scala/org/apache/spark/rdd/RDD.scala
##########
@@ -1233,6 +1233,22 @@ abstract class RDD[T: ClassTag](
           (i, iter) => iter.map((i % curNumPartitions, _))

Review comment:
       yes, we can add a new method with signature that looks like this: 
   ```
     def treeAggregate[U](
       zeroValue: U,
       seqOp: JFunction2[U, T, U],
       combOp: JFunction2[U, U, U],
       depth: Int,
       finalAggregateOnExecutor: Boolean): U = {
   ...
   ```
   you're right, we dont need to worry about Mima excludes then. 
   but that is not the problem. the problem is that this method will need to be 
overloaded by the other treeAggregate methods. for example, the other 
treeAggregate methods will have to call this version of treeAggregate. this is 
an issue because of zeroValue:U is only inspected by the compiler to resolve 
the overloaded methods. Due to that the compiler throws ambiguous reference to 
overloaded definition, and this cannot be resolved..
   
   there are only 2 workarounds: 
   1. create a new method with a different name but same signature with the new 
parameter. something like `treeAggregateExecutor` and have somewhat duplicate 
code in that method. me and @venkata91  thought this approach was kind of ugly
   2. use config  approach 




-- 
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]

Reply via email to