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

    https://github.com/apache/spark/pull/23062#discussion_r234834427
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala 
---
    @@ -788,12 +788,37 @@ object ScalaReflection extends ScalaReflection {
       }
     
       /**
    -   * Finds an accessible constructor with compatible parameters. This is a 
more flexible search
    -   * than the exact matching algorithm in `Class.getConstructor`. The 
first assignment-compatible
    -   * matching constructor is returned. Otherwise, it returns `None`.
    +   * Finds an accessible constructor with compatible parameters. This is a 
more flexible search than
    +   * the exact matching algorithm in `Class.getConstructor`. The first 
assignment-compatible
    +   * matching constructor is returned if it exists. Otherwise, we check 
for additional compatible
    +   * constructors defined in the companion object as `apply` methods. 
Otherwise, it returns `None`.
        */
    -  def findConstructor(cls: Class[_], paramTypes: Seq[Class[_]]): 
Option[Constructor[_]] = {
    -    Option(ConstructorUtils.getMatchingAccessibleConstructor(cls, 
paramTypes: _*))
    +  def findConstructor(cls: Class[_], paramTypes: Seq[Class[_]]): 
Option[Seq[AnyRef] => Any] = {
    --- End diff --
    
    I guess externally `findConstructor` would then be cleaner though? It's too 
bad it is only called in one place.


---

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

Reply via email to