HyukjinKwon commented on a change in pull request #28633:
URL: https://github.com/apache/spark/pull/28633#discussion_r429805059



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala
##########
@@ -312,6 +312,10 @@ case object NamePlaceholder extends LeafExpression with 
Unevaluable {
  * Returns a Row containing the evaluation of all children expressions.
  */
 object CreateStruct extends FunctionBuilder {
+  /**
+   * Returns a named struct with generating names or using the names when 
available.
+   * It should be used only for an internal purpose.

Review comment:
       There are some cases when `CreateNamedStruct` is inserted, e.g.) 
https://github.com/apache/spark/pull/28633#discussion_r429756482
   
   I was thinking that it's fine to treat the cases as just using 
`named_struct` internally. Also, it makes many diff in SQL tests and etc. 
Thought it's better to minimise the diff.
   
   Actually moving to `CreateNamedStruct` is the first way I tried. One thing 
is that `CreateNamedStruct` case class has the same signature so 
`CreateNamedStruct` companion object can't have the same signature at `apply`.
   
   I could consistently have `CreateNamedStruct.create` and 
`CreateStruct.create` if you prefer this way.




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

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