Skip to content

Conversation

@marmbrus
Copy link
Contributor

Before this PR there were two things that would blow up if you called df.as[MyClass] if MyClass was defined in the REPL:

  • Because classForName doesn't work on the munged names returned by tpe.erasure.typeSymbol.asClass.fullName
  • Because we don't have anything to pass into the constructor for the $outer pointer.

Note that this PR is just adding the infrastructure for working with inner classes in encoder and is not yet sufficient to make them work in the REPL. Currently, the implementation show in marmbrus@95cec7d is causing a bug that breaks code gen due to some interaction between janino and the ExecutorClassLoader. This will be addressed in a follow-up PR.

@SparkQA
Copy link

SparkQA commented Nov 10, 2015

Test build #45543 has finished for PR 9602 at commit a0a04a7.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n

@hvanhovell
Copy link
Contributor

Does a REPL defined class blow up with a java.lang.InternalError? If it does, then we have the same problem: #9568

@marmbrus
Copy link
Contributor Author

Interesting, I don't think thats it though. We are lacking the outer pointer when we try to construct it inside of the encoder.

@marmbrus marmbrus force-pushed the dataset-replClasses branch from a0a04a7 to fe657af Compare November 17, 2015 07:46
@marmbrus marmbrus changed the title [WIP] [SPARK-11636] [SQL] Support as for classes defined in the REPL [SPARK-11636] [SQL] Support as for classes defined in the REPL Nov 17, 2015
@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46068 has finished for PR 9602 at commit fe657af.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n

@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46072 has finished for PR 9602 at commit cb60d16.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n

@marmbrus marmbrus force-pushed the dataset-replClasses branch 2 times, most recently from ef65e9e to b52ef46 Compare November 18, 2015 02:55
@marmbrus marmbrus force-pushed the dataset-replClasses branch from b52ef46 to 35cd4f4 Compare November 18, 2015 02:55
@marmbrus marmbrus changed the title [SPARK-11636] [SQL] Support as for classes defined in the REPL [SPARK-11636] [SQL] Support classes defined in the REPL with Encoders Nov 18, 2015
@SparkQA
Copy link

SparkQA commented Nov 18, 2015

Test build #46150 has finished for PR 9602 at commit cac62c7.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n

@SparkQA
Copy link

SparkQA commented Nov 18, 2015

Test build #46142 has finished for PR 9602 at commit 59ca3ce.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * val fullLine = if (line contains \" class \") \"\" + line else line\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n

@SparkQA
Copy link

SparkQA commented Nov 18, 2015

Test build #46152 has finished for PR 9602 at commit 35cd4f4.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n

@SparkQA
Copy link

SparkQA commented Nov 18, 2015

Test build #46167 has finished for PR 9602 at commit e38999b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n

@marmbrus
Copy link
Contributor Author

Test this please

Conflicts:
	sql/core/src/main/scala/org/apache/spark/sql/GroupedDataset.scala
@marmbrus marmbrus force-pushed the dataset-replClasses branch from 2049053 to fa96708 Compare November 18, 2015 21:44
@SparkQA
Copy link

SparkQA commented Nov 18, 2015

Test build #46258 has finished for PR 9602 at commit 1773e08.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n * s\"Unable to generate an encoder for inner class$`\n

@SparkQA
Copy link

SparkQA commented Nov 19, 2015

Test build #2089 has finished for PR 9602 at commit 95cec7d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ScalaReflection\n * case class Schema(dataType: DataType, nullable: Boolean)\n

asfgit pushed a commit that referenced this pull request Nov 19, 2015
Before this PR there were two things that would blow up if you called `df.as[MyClass]` if `MyClass` was defined in the REPL:
 - [x] Because `classForName` doesn't work on the munged names returned by `tpe.erasure.typeSymbol.asClass.fullName`
 - [x] Because we don't have anything to pass into the constructor for the `$outer` pointer.

Note that this PR is just adding the infrastructure for working with inner classes in encoder and is not yet sufficient to make them work in the REPL.  Currently, the implementation show in marmbrus@95cec7d is causing a bug that breaks code gen due to some interaction between janino and the `ExecutorClassLoader`.  This will be addressed in a follow-up PR.

Author: Michael Armbrust <[email protected]>

Closes #9602 from marmbrus/dataset-replClasses.

(cherry picked from commit 59a5013)
Signed-off-by: Michael Armbrust <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants