[SPARK-11819][SQL] nice error message for missing encoder #9810

cloud-fan · 2015-11-18T15:10:28Z

before this PR, when users try to get an encoder for an un-supported class, they will only get a very simple error message like Encoder for type xxx is not supported.

After this PR, the error message become more friendly, for example:

No Encoder found for abc.xyz.NonEncodable
- array element class: "abc.xyz.NonEncodable"
- field (class: "scala.Array", name: "arrayField")
- root class: "abc.xyz.AnotherClass"

cloud-fan · 2015-11-18T15:12:35Z

cc @marmbrus

SparkQA · 2015-11-18T15:36:11Z

Test build #46217 has finished for PR 9810 at commit 3d9fcc9.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-11-18T18:03:13Z

Test build #46220 has finished for PR 9810 at commit 8d8d857.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

marmbrus · 2015-11-18T18:10:12Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala

scala doc please

marmbrus · 2015-11-18T18:13:32Z

This message looks great!

rxin · 2015-11-18T21:38:43Z

This is really cool.

cloud-fan · 2015-11-19T13:32:44Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala

Any suggestions for the error message format? cc @marmbrus @rxin

I actually think this looks pretty good.

SparkQA · 2015-11-19T13:43:16Z

Test build #46328 has finished for PR 9810 at commit da13931.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-11-19T15:00:51Z

Test build #46329 has finished for PR 9810 at commit ddb092a.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2015-11-19T15:08:23Z

retest this please.

SparkQA · 2015-11-19T17:00:29Z

Test build #46335 has finished for PR 9810 at commit ddb092a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

marmbrus · 2015-11-19T19:59:29Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala

Why is this a separate function now? Now we have duplicated copies of the same logic that can fall out of sync.

I tried but stuck with array handling. For array we use MapObjects and call extractorFor lazily as a function parameter. Any ideas for it?

An initial eager call with a dummy attribute reference?

sounds good, let me have a try

SparkQA · 2015-11-20T04:50:53Z

Test build #46385 has finished for PR 9810 at commit c263c14.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

marmbrus · 2015-11-20T20:04:23Z

Thanks, merging to master and 1.6.

before this PR, when users try to get an encoder for an un-supported class, they will only get a very simple error message like `Encoder for type xxx is not supported`. After this PR, the error message become more friendly, for example: ``` No Encoder found for abc.xyz.NonEncodable - array element class: "abc.xyz.NonEncodable" - field (class: "scala.Array", name: "arrayField") - root class: "abc.xyz.AnotherClass" ``` Author: Wenchen Fan <wenchen@databricks.com> Closes #9810 from cloud-fan/error-message. (cherry picked from commit 3b9d2a3) Signed-off-by: Michael Armbrust <michael@databricks.com>

JoshRosen · 2015-11-20T21:01:30Z

I think that this doesn't work on Scala 2.11: https://amplab.cs.berkeley.edu/jenkins/view/Spark-QA-Compile/job/Spark-Master-Scala211-Compile/2156/console

cloud-fan · 2015-11-21T04:43:10Z

@JoshRosen , I think the reason maybe that, I defined some private methods in trait ScalaReflection and use them in object ScalaReflection extends ScalaReflection, seems scala 2.11 doesn't support this, let me change those methods to public and see if it works.

marmbrus reviewed Nov 18, 2015
View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala Outdated

Copy link

Contributor

marmbrus Nov 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scala doc please

cloud-fan force-pushed the error-message branch from 8d8d857 to da13931 Compare November 19, 2015 13:29

cloud-fan reviewed Nov 19, 2015
View reviewed changes

cloud-fan force-pushed the error-message branch from da13931 to ddb092a Compare November 19, 2015 13:57

marmbrus reviewed Nov 19, 2015
View reviewed changes

nice error message for missing encoder

c263c14

cloud-fan force-pushed the error-message branch from ddb092a to c263c14 Compare November 20, 2015 02:31

asfgit closed this in 3b9d2a3 Nov 20, 2015

cloud-fan deleted the error-message branch November 21, 2015 04:44

[SPARK-11819][SQL] nice error message for missing encoder #9810

[SPARK-11819][SQL] nice error message for missing encoder #9810

Uh oh!

Conversation

cloud-fan commented Nov 18, 2015

Uh oh!

cloud-fan commented Nov 18, 2015

Uh oh!

SparkQA commented Nov 18, 2015

Uh oh!

SparkQA commented Nov 18, 2015

Uh oh!

marmbrus Nov 18, 2015

Choose a reason for hiding this comment

Uh oh!

marmbrus commented Nov 18, 2015

Uh oh!

rxin commented Nov 18, 2015

Uh oh!

cloud-fan Nov 19, 2015

Choose a reason for hiding this comment

Uh oh!

marmbrus Nov 19, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 19, 2015

Uh oh!

SparkQA commented Nov 19, 2015

Uh oh!

cloud-fan commented Nov 19, 2015

Uh oh!

SparkQA commented Nov 19, 2015

Uh oh!

marmbrus Nov 19, 2015

Choose a reason for hiding this comment

Uh oh!

cloud-fan Nov 20, 2015

Choose a reason for hiding this comment

Uh oh!

marmbrus Nov 20, 2015

Choose a reason for hiding this comment

Uh oh!

cloud-fan Nov 20, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 20, 2015

Uh oh!

marmbrus commented Nov 20, 2015

Uh oh!

JoshRosen commented Nov 20, 2015

Uh oh!

cloud-fan commented Nov 21, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants