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


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -67,7 +67,7 @@ class SparkConnectPlanner(plan: proto.Relation, session: 
SparkSession) {
       case proto.Relation.RelTypeCase.AGGREGATE => 
transformAggregate(rel.getAggregate)
       case proto.Relation.RelTypeCase.SQL => transformSql(rel.getSql)
       case proto.Relation.RelTypeCase.LOCAL_RELATION =>
-        transformLocalRelation(rel.getLocalRelation)
+        transformLocalRelation(rel.getLocalRelation, common)

Review Comment:
   This is legacy design that I believe it thinks only relations have the 
optional alias. 
   
   Every logical plan could have an optional alias, in that case I prefer to 
move that alias out of the `common` to have its own message. This is because by 
that we can differentiate
   ```
   .xx()
   .xx().as("") // probably invalid but user can write down such API
   .xx().as("alias_1")
   ```  
   
   I can also change this in this PR if you think this is a right time.



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