hvanhovell commented on code in PR #49111:
URL: https://github.com/apache/spark/pull/49111#discussion_r1878795333


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/KeyValueGroupedDataset.scala:
##########
@@ -460,14 +460,20 @@ private class KeyValueGroupedDatasetImpl[K, V, IK, IV](
   }
 
   override protected def aggUntyped(columns: TypedColumn[_, _]*): Dataset[_] = 
{
-    // TODO(SPARK-43415): For each column, apply the valueMap func first...
+    val aggCols: Seq[Column] = valueMapFunc match {

Review Comment:
   @xupefei why do we need custom handling on the server side? I dislike that 
because it is something very scala specific that is now part of the public API. 
Yes we have accidentally done that in the past, but that does not make it a 
good idea :)...
   
   Can we instead create a projection of the form `select(keys..., 
fn(col(*)).as("value)"))` and rewrite the agg exprs to use the value column as 
its input in the `ColumnNodeToProtoConverter` (`col(*)` would have to be 
rewritten to `col (value)`)?
   
   



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to