[GitHub] [spark] amaliujia commented on a diff in pull request #38838: [SPARK-41321][CONNECT] Support target field for UnresolvedStar

[GitBox <gi...@apache.org>]

amaliujia commented on code in PR #38838:
URL: https://github.com/apache/spark/pull/38838#discussion_r1035593027


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -360,13 +360,8 @@ class SparkConnectPlanner(session: SparkSession) {
     } else {
       logical.OneRowRelation()
     }
-    // TODO: support the target field for *.
     val projection =
-      if (rel.getExpressionsCount == 1 && rel.getExpressions(0).hasUnresolvedStar) {
-        Seq(UnresolvedStar(Option.empty))
-      } else {
-        rel.getExpressionsList.asScala.map(transformExpression).map(UnresolvedAlias(_))
-      }
+      rel.getExpressionsList.asScala.map(transformExpression).map(UnresolvedAlias(_))

Review Comment:
   Though the code was written like this, I am seeing a problem that if the Expression is already a `Alias`, we should not wrap it into `UnresolvedAlias` again? 
   
   I think you can write a test case like `SELECT 1 as col` then you will find the output schema is not `col` but a auto-generated alias.



##########
connector/connect/src/main/protobuf/spark/connect/expressions.proto:
##########
@@ -142,6 +141,9 @@ message Expression {
 
   // UnresolvedStar is used to expand all the fields of a relation or struct.
   message UnresolvedStar {
+    // The target of the expansion, either be a table name or struct name, this

Review Comment:
   For our current proto style guide. Each field should be marked as either `(Required) or `(Optional)` at the beginning of its comment. See: https://github.com/apache/spark/blob/master/connector/connect/docs/adding-proto-messages.md



-- 
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-unsubscribe@spark.apache.org

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


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