HyukjinKwon commented on code in PR #47688: URL: https://github.com/apache/spark/pull/47688#discussion_r1719253652
########## sql/core/src/main/scala/org/apache/spark/sql/Column.scala: ########## @@ -27,22 +27,21 @@ import org.apache.spark.sql.catalyst.encoders.{encoderFor, ExpressionEncoder} import org.apache.spark.sql.catalyst.expressions._ import org.apache.spark.sql.catalyst.expressions.aggregate.AggregateExpression import org.apache.spark.sql.catalyst.parser.CatalystSqlParser -import org.apache.spark.sql.catalyst.util.{toPrettySQL, CharVarcharUtils} +import org.apache.spark.sql.catalyst.util.toPrettySQL import org.apache.spark.sql.execution.aggregate.TypedAggregateExpression -import org.apache.spark.sql.execution.analysis.DetectAmbiguousSelfJoin import org.apache.spark.sql.expressions.Window import org.apache.spark.sql.functions.lit -import org.apache.spark.sql.internal.TypedAggUtils +import org.apache.spark.sql.internal.{ColumnNode, TypedAggUtils, Wrapper} import org.apache.spark.sql.types._ import org.apache.spark.util.ArrayImplicits._ -private[sql] object Column { +private[spark] object Column { def apply(colName: String): Column = new Column(colName) - def apply(expr: Expression): Column = new Column(expr) + def apply(expr: Expression): Column = Column(Wrapper(expr)) Review Comment: Wonder if we should have a different name from `Wrapper` (with some scaladoc for that class). To me, it's a bit difficult to follow why we have `Wrapper` and what it dose. -- 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