Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19480#discussion_r147060840
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
 ---
    @@ -78,6 +79,22 @@ case class SubExprEliminationState(isNull: String, 
value: String)
     case class SubExprCodes(codes: Seq[String], states: Map[Expression, 
SubExprEliminationState])
     
     /**
    + * The main information about a new added function.
    + *
    + * @param functionName String representing the name of the function
    + * @param subclassName Optional value which is empty if the function is 
added to
    + *                     the outer class, otherwise it contains the name of 
the
    + *                     inner class in which the function has been added.
    + * @param subclassInstance Optional value which is empty if the function 
is added to
    --- End diff --
    
    I saw three ways to represent the same concept. subclass, inner class, 
nested classes. 
    
    How about renaming all of them to inner classes? Could you go over all the 
code changes in this PR to make them consistent?


---

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

Reply via email to