You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by michalsenkyr <gi...@git.apache.org> on 2018/02/04 23:17:55 UTC

[GitHub] spark pull request #20505: [SPARK-23251][SQL] Add checks for collection elem...

GitHub user michalsenkyr opened a pull request:

    https://github.com/apache/spark/pull/20505

    [SPARK-23251][SQL] Add checks for collection element Encoders

    Implicit methods of `SQLImplicits` providing Encoders for collections did not check for
    Encoders for their elements. This resulted in unrelated error messages during run-time instead of proper failures during compilation.
    
    ## What changes were proposed in this pull request?
    
    `Seq`, `Map` and `Set` `Encoder` providers' type parameters and implicit parameters were modified to facilitate the appropriate checks.
    
    Unfortunately, this doesn't result in the desired "Unable to find encoder for type stored in a Dataset" error as the Scala compiler generates a different error when an implicit cannot be found due to missing arguments (see [ContextErrors](https://github.com/scala/scala/blob/v2.11.12/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala#L77)). `@implicitNotFound` is not used in this case.
    
    ## How was this patch tested?
    
    New behavior:
    ```scala
    scala> implicitly[Encoder[Map[String, Any]]]
    <console>:25: error: diverging implicit expansion for type org.apache.spark.sql.Encoder[Map[String,Any]]
    starting with method newStringEncoder in class SQLImplicits
           implicitly[Encoder[Map[String, Any]]]
                     ^
    ```
    
    Old behavior:
    ```scala
    scala> implicitly[Encoder[Map[String, Any]]]
    java.lang.ClassNotFoundException: scala.Any
      at scala.reflect.internal.util.AbstractFileClassLoader.findClass(AbstractFileClassLoader.scala:62)
      at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
      at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
      at java.lang.Class.forName0(Native Method)
      at java.lang.Class.forName(Class.java:348)
      at scala.reflect.runtime.JavaMirrors$JavaMirror.javaClass(JavaMirrors.scala:555)
      at scala.reflect.runtime.JavaMirrors$JavaMirror$$anonfun$classToJava$1.apply(JavaMirrors.scala:1211)
      at scala.reflect.runtime.JavaMirrors$JavaMirror$$anonfun$classToJava$1.apply(JavaMirrors.scala:1203)
      at scala.reflect.runtime.TwoWayCaches$TwoWayCache$$anonfun$toJava$1.apply(TwoWayCaches.scala:49)
      at scala.reflect.runtime.Gil$class.gilSynchronized(Gil.scala:19)
      at scala.reflect.runtime.JavaUniverse.gilSynchronized(JavaUniverse.scala:16)
      at scala.reflect.runtime.TwoWayCaches$TwoWayCache.toJava(TwoWayCaches.scala:44)
      at scala.reflect.runtime.JavaMirrors$JavaMirror.classToJava(JavaMirrors.scala:1203)
      at scala.reflect.runtime.JavaMirrors$JavaMirror.runtimeClass(JavaMirrors.scala:194)
      at scala.reflect.runtime.JavaMirrors$JavaMirror.runtimeClass(JavaMirrors.scala:54)
      at org.apache.spark.sql.catalyst.ScalaReflection$.getClassFromType(ScalaReflection.scala:700)
      at org.apache.spark.sql.catalyst.ScalaReflection$$anonfun$org$apache$spark$sql$catalyst$ScalaReflection$$dataTypeFor$1.apply(ScalaReflection.scala:84)
      at org.apache.spark.sql.catalyst.ScalaReflection$$anonfun$org$apache$spark$sql$catalyst$ScalaReflection$$dataTypeFor$1.apply(ScalaReflection.scala:65)
      at scala.reflect.internal.tpe.TypeConstraints$UndoLog.undo(TypeConstraints.scala:56)
      at org.apache.spark.sql.catalyst.ScalaReflection$class.cleanUpReflectionObjects(ScalaReflection.scala:824)
      at org.apache.spark.sql.catalyst.ScalaReflection$.cleanUpReflectionObjects(ScalaReflection.scala:39)
      at org.apache.spark.sql.catalyst.ScalaReflection$.org$apache$spark$sql$catalyst$ScalaReflection$$dataTypeFor(ScalaReflection.scala:64)
      at org.apache.spark.sql.catalyst.ScalaReflection$$anonfun$org$apache$spark$sql$catalyst$ScalaReflection$$serializerFor$1.apply(ScalaReflection.scala:512)
      at org.apache.spark.sql.catalyst.ScalaReflection$$anonfun$org$apache$spark$sql$catalyst$ScalaReflection$$serializerFor$1.apply(ScalaReflection.scala:445)
      at scala.reflect.internal.tpe.TypeConstraints$UndoLog.undo(TypeConstraints.scala:56)
      at org.apache.spark.sql.catalyst.ScalaReflection$class.cleanUpReflectionObjects(ScalaReflection.scala:824)
      at org.apache.spark.sql.catalyst.ScalaReflection$.cleanUpReflectionObjects(ScalaReflection.scala:39)
      at org.apache.spark.sql.catalyst.ScalaReflection$.org$apache$spark$sql$catalyst$ScalaReflection$$serializerFor(ScalaReflection.scala:445)
      at org.apache.spark.sql.catalyst.ScalaReflection$.serializerFor(ScalaReflection.scala:434)
      at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder$.apply(ExpressionEncoder.scala:71)
      at org.apache.spark.sql.SQLImplicits.newMapEncoder(SQLImplicits.scala:172)
      ... 49 elided
    ```

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/michalsenkyr/spark SPARK-23251

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/20505.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #20505
    
----
commit e3d6dbfdf6af274090e3ba6cc5795745600d26c1
Author: Michal Senkyr <mi...@...>
Date:   2018-02-04T22:08:25Z

    Add checks for collection element Encoders
    
    Implicit methods providing Encoders for collections did not check for
    element Encoders, making related operations fail during run-time.

commit d2167607874c574961c490f188ca5c618a6864be
Author: Michal Senkyr <mi...@...>
Date:   2018-02-04T23:15:31Z

    Add unit tests

----


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by michalsenkyr <gi...@git.apache.org>.
Github user michalsenkyr commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    Yes, that is the idea. Frankly, I am not that familiar with how the compiler resolves all the implicit parameters to say confidently what is going on. But here's my take:
    
    I did a little more research and found out that the "diverging implicit expansion" error means the compiler is able to follow a resolution route that can be potentially infinite. I think it might be possible that the compiler is following my other implicit methods, trying to fit various collections inside V with the hope of satisfying the `<:<` condition some time in the future before finally giving up.
    
    Just to be sure multi-level collections work with my change, I successfully tried this:
    ```
    scala> implicitly[Encoder[Map[Seq[Map[String, Seq[Long]]], List[Array[Map[String, Int]]]]]]
    res5: org.apache.spark.sql.Encoder[Map[Seq[Map[String,Seq[Long]]],List[Array[Map[String,Int]]]]] = class[value[0]: map<array<map<string,array<bigint>>>,array<array<map<string,int>>>>]
    ```
    
    One thing that doesn't make sense for me, however, is the information the compiler gives me when enabling implicit resolution logging via `-Xlog-implicits`:
    ```
    scala> implicitly[Encoder[Map[String, Any]]]
    <console>:25: newCheckedSetEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[Map[String,Any]] because:
    hasMatchingSymbol reported error: polymorphic expression cannot be instantiated to expected type;
     found   : [T[_], E]org.apache.spark.sql.Encoder[T[E]]
     required: org.apache.spark.sql.Encoder[Map[String,Any]]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedSetEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[String] because:
    hasMatchingSymbol reported error: polymorphic expression cannot be instantiated to expected type;
     found   : [T[_], E]org.apache.spark.sql.Encoder[T[E]]
     required: org.apache.spark.sql.Encoder[String]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedMapEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[String] because:
    hasMatchingSymbol reported error: polymorphic expression cannot be instantiated to expected type;
     found   : [T[_, _], K, V]org.apache.spark.sql.Encoder[T[K,V]]
     required: org.apache.spark.sql.Encoder[String]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedSequenceEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[String] because:
    hasMatchingSymbol reported error: polymorphic expression cannot be instantiated to expected type;
     found   : [T[_], E]org.apache.spark.sql.Encoder[T[E]]
     required: org.apache.spark.sql.Encoder[String]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: materializing requested reflect.runtime.universe.type.TypeTag[A] using `package`.this.materializeTypeTag[A](scala.reflect.runtime.`package`.universe)
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedMapEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[E] because:
    hasMatchingSymbol reported error: diverging implicit expansion for type org.apache.spark.sql.Encoder[K]
    starting with method newStringEncoder in class SQLImplicits
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedSetEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[Any] because:
    hasMatchingSymbol reported error: ambiguous implicit values:
     both method newIntEncoder in class SQLImplicits of type => org.apache.spark.sql.Encoder[Int]
     and method newLongEncoder in class SQLImplicits of type => org.apache.spark.sql.Encoder[Long]
     match expected type org.apache.spark.sql.Encoder[E]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedMapEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[Any] because:
    hasMatchingSymbol reported error: diverging implicit expansion for type org.apache.spark.sql.Encoder[K]
    starting with method newStringEncoder in class SQLImplicits
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: materializing requested reflect.runtime.universe.type.TypeTag[A] using `package`.this.materializeTypeTag[A](scala.reflect.runtime.`package`.universe)
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedMapEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[E] because:
    hasMatchingSymbol reported error: diverging implicit expansion for type org.apache.spark.sql.Encoder[K]
    starting with method newStringEncoder in class SQLImplicits
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedSequenceEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[Any] because:
    hasMatchingSymbol reported error: ambiguous implicit values:
     both method newIntEncoder in class SQLImplicits of type => org.apache.spark.sql.Encoder[Int]
     and method newLongEncoder in class SQLImplicits of type => org.apache.spark.sql.Encoder[Long]
     match expected type org.apache.spark.sql.Encoder[E]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: materializing requested reflect.runtime.universe.type.TypeTag[Any] using `package`.this.materializeTypeTag[Any](scala.reflect.runtime.`package`.universe)
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newProductEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[Any] because:
    typing TypeApply reported errors for the implicit tree: type arguments [Any] do not conform to method newProductEncoder's type parameter bounds [T <: Product]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedMapEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[Map[String,Any]] because:
    hasMatchingSymbol reported error: diverging implicit expansion for type org.apache.spark.sql.Encoder[K]
    starting with method newStringEncoder in class SQLImplicits
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newCheckedSequenceEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[Map[String,Any]] because:
    hasMatchingSymbol reported error: polymorphic expression cannot be instantiated to expected type;
     found   : [T[_], E]org.apache.spark.sql.Encoder[T[E]]
     required: org.apache.spark.sql.Encoder[Map[String,Any]]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: materializing requested reflect.runtime.universe.type.TypeTag[Map[String,Any]] using `package`.this.materializeTypeTag[Map[String,Any]](scala.reflect.runtime.`package`.universe)
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: newProductEncoder is not a valid implicit value for org.apache.spark.sql.Encoder[Map[String,Any]] because:
    typing TypeApply reported errors for the implicit tree: type arguments [Map[String,Any]] do not conform to method newProductEncoder's type parameter bounds [T <: Product]
           implicitly[Encoder[Map[String, Any]]]
                     ^
    <console>:25: error: diverging implicit expansion for type org.apache.spark.sql.Encoder[Map[String,Any]]
    starting with method newStringEncoder in class SQLImplicits
           implicitly[Encoder[Map[String, Any]]]
    ```
    Note the "ambiguous implicit values" errors on Long/Int Encoders while looking for Set/Seq Encoders. I really can't explain what led the compiler to consider those. I'll look some more and try to find out exactly what is going on there, but I am suspecting it might have something to do with covariance on collection parameters.


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    **[Test build #87060 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87060/testReport)** for PR 20505 at commit [`d216760`](https://github.com/apache/spark/commit/d2167607874c574961c490f188ca5c618a6864be).
     * This patch **fails MiMa tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    ok to test


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    **[Test build #87074 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87074/testReport)** for PR 20505 at commit [`cd7a1ff`](https://github.com/apache/spark/commit/cd7a1fffb76b7b6e2727bff2219247f868f58565).


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    **[Test build #87074 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87074/testReport)** for PR 20505 at commit [`cd7a1ff`](https://github.com/apache/spark/commit/cd7a1fffb76b7b6e2727bff2219247f868f58565).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark pull request #20505: [SPARK-23251][SQL] Add checks for collection elem...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20505#discussion_r165889177
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/SQLImplicits.scala ---
    @@ -165,11 +165,15 @@ abstract class SQLImplicits extends LowPrioritySQLImplicits {
       def newProductSeqEncoder[A <: Product : TypeTag]: Encoder[Seq[A]] = ExpressionEncoder()
     
       /** @since 2.2.0 */
    -  implicit def newSequenceEncoder[T <: Seq[_] : TypeTag]: Encoder[T] = ExpressionEncoder()
    +  implicit def newSequenceEncoder[T[_], E : Encoder]
    --- End diff --
    
    Can you check if we are breaking some public contract here?


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87060/
    Test FAILed.


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    **[Test build #87060 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87060/testReport)** for PR 20505 at commit [`d216760`](https://github.com/apache/spark/commit/d2167607874c574961c490f188ca5c618a6864be).


---

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


[GitHub] spark pull request #20505: [SPARK-23251][SQL] Add checks for collection elem...

Posted by michalsenkyr <gi...@git.apache.org>.
Github user michalsenkyr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20505#discussion_r165903346
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/SQLImplicits.scala ---
    @@ -165,11 +165,15 @@ abstract class SQLImplicits extends LowPrioritySQLImplicits {
       def newProductSeqEncoder[A <: Product : TypeTag]: Encoder[Seq[A]] = ExpressionEncoder()
     
       /** @since 2.2.0 */
    -  implicit def newSequenceEncoder[T <: Seq[_] : TypeTag]: Encoder[T] = ExpressionEncoder()
    +  implicit def newSequenceEncoder[T[_], E : Encoder]
    --- End diff --
    
    Looks like we are. I can add new methods and make the old ones not implicit. That should fix MiMa. Although that might add to the clutter that's already in this class. Is that OK? 


---

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


[GitHub] spark pull request #20505: [SPARK-23251][SQL] Add checks for collection elem...

Posted by michalsenkyr <gi...@git.apache.org>.
Github user michalsenkyr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20505#discussion_r167437023
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/SQLImplicits.scala ---
    @@ -165,11 +165,15 @@ abstract class SQLImplicits extends LowPrioritySQLImplicits {
       def newProductSeqEncoder[A <: Product : TypeTag]: Encoder[Seq[A]] = ExpressionEncoder()
     
       /** @since 2.2.0 */
    -  implicit def newSequenceEncoder[T <: Seq[_] : TypeTag]: Encoder[T] = ExpressionEncoder()
    +  implicit def newSequenceEncoder[T[_], E : Encoder]
    --- End diff --
    
    Just letting you know that I updated this PR with the above fix. MiMa is now passing.


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87074/
    Test PASSed.


---

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


[GitHub] spark issue #20505: [SPARK-23251][SQL] Add checks for collection element Enc...

Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:

    https://github.com/apache/spark/pull/20505
  
    I am trying to understand the web that the compiler follows to satisfy the implicit Encoder parameter.
    
    The DataSet’s map function is declared as follows:
    
    <code>def map[U : Encoder](func: T => U): Dataset[U]</code>
    
    There is an implicit parameter of type Encoder[U], where U is the return type of the passed function.
    
    Then there is my application code:
    
    <pre>
    df.map(row => row.getValuesMap[Any](List("stationName", "year"))).collect 
    </pre>
    
    This causes the compiler to look for an instance of an Encoder[Map[String, Any]] in scope.
    
    Before your change, the compiler used to find:
    
    <code>implicit def newMapEncoder[T <: Map[_, _] : TypeTag]: Encoder[T] = ExpressionEncoder()</code>
    
    This finds an encoder for Map, but it fails to verify that the map’s type parameters (in this case, String and Any) also have encoders. Therefore the code compiles, but the map() method fails at runtime with a mystery exception.
    
    You removed the “implicit” keyword from above, so now the compiler finds your new definition instead:
    
    <pre>implicit def newCheckedMapEncoder[T[_, _], K : Encoder, V : Encoder]
    (implicit ev: T[K, V] <:< Map[K, V], tag: TypeTag[T[K, V]]): Encoder[T[K, V]]
    </pre>
    
    This method is parameterized with any sort of type that is itself parameterized with two types. However, your evidence parameter <code>ev: T[K, V] <:< Map[K, V]</code> restricts the match to Map types of K and V.
    
    You also have two unnamed implicit parameters of type Encoder[K] and Encoder[V] that caused the compiler to start a new search for instances of Encoder[K] and Encoder[V] that are in scope.
    
    The compiler surely finds an instance of Encoder[String], but fails to find an instance of Encoder[Any], so the compilation fails.
    
    I assume the compiler prints that odd error message ("diverging implicit expansion") because it failed during this second search, rather than on the search that started the whole thing.


---

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