From 4ad5b7f40f1393c9894c3d1dcf4ef1e0e918d4eb Mon Sep 17 00:00:00 2001 From: Wenchen Fan Date: Thu, 2 Jun 2016 17:03:34 -0700 Subject: [PATCH 1/2] better error message when use java reserved keyword as field name --- .../apache/spark/sql/catalyst/ScalaReflection.scala | 11 +++++++++++ .../scala/org/apache/spark/sql/DatasetSuite.scala | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala index 052cc486e8cb2..8259f15423471 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala @@ -582,6 +582,11 @@ object ScalaReflection extends ScalaReflection { case t if definedByConstructorParams(t) => val params = getConstructorParameters(t) val nonNullOutput = CreateNamedStruct(params.flatMap { case (fieldName, fieldType) => + if (javaKeywords.contains(fieldName)) { + throw new UnsupportedOperationException(s"`$fieldName` is a java reserved keyword " + + "and can not be used as field name\n" + walkedTypePath.mkString("\n")) + } + val fieldValue = Invoke(inputObject, fieldName, dataTypeFor(fieldType)) val clsName = getClassNameFromType(fieldType) val newPath = s"""- field (class: "$clsName", name: "$fieldName")""" +: walkedTypePath @@ -720,6 +725,12 @@ object ScalaReflection extends ScalaReflection { tpe <:< localTypeOf[Product] || tpe <:< localTypeOf[DefinedByConstructorParams] } + private val javaKeywords = Set("abstract", "assert", "boolean", "break", "byte", "case", "catch", + "char", "class", "const", "continue", "default", "do", "double", "else", "extends", "false", + "final", "finally", "float", "for", "goto", "if", "implements", "import", "instanceof", "int", + "interface", "long", "native", "new", "null", "package", "private", "protected", "public", + "return", "short", "static", "strictfp", "super", "switch", "synchronized", "this", "throw", + "throws", "transient", "true", "try", "void", "volatile", "while") } /** diff --git a/sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala index a3881ff920159..f8e1ebbc9fa02 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala @@ -786,6 +786,14 @@ class DatasetSuite extends QueryTest with SharedSQLContext { val result = joined.collect().toSet assert(result == Set(ClassData("a", 1) -> null, ClassData("b", 2) -> ClassData("x", 2))) } + + test("better error message when use java reserved keyword as field name") { + val e = intercept[UnsupportedOperationException] { + Seq(InvalidInJava(1)).toDS() + } + assert(e.getMessage.contains( + "`abstract` is a java reserved keyword and can not be used as field name")) + } } case class Generic[T](id: T, value: Double) @@ -809,6 +817,8 @@ case class ClassNullableData(a: String, b: Integer) case class NestedStruct(f: ClassData) case class DeepNestedStruct(f: NestedStruct) +case class InvalidInJava(`abstract`: Int) + /** * A class used to test serialization using encoders. This class throws exceptions when using * Java serialization -- so the only way it can be "serialized" is through our encoders. From 51d403dd632e81d9ebf0f943c2fb6be45edf50f3 Mon Sep 17 00:00:00 2001 From: Wenchen Fan Date: Thu, 2 Jun 2016 17:19:01 -0700 Subject: [PATCH 2/2] address comments --- .../scala/org/apache/spark/sql/catalyst/ScalaReflection.scala | 4 ++-- .../src/test/scala/org/apache/spark/sql/DatasetSuite.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala index 8259f15423471..47508618178ea 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala @@ -583,8 +583,8 @@ object ScalaReflection extends ScalaReflection { val params = getConstructorParameters(t) val nonNullOutput = CreateNamedStruct(params.flatMap { case (fieldName, fieldType) => if (javaKeywords.contains(fieldName)) { - throw new UnsupportedOperationException(s"`$fieldName` is a java reserved keyword " + - "and can not be used as field name\n" + walkedTypePath.mkString("\n")) + throw new UnsupportedOperationException(s"`$fieldName` is a reserved keyword and " + + "cannot be used as field name\n" + walkedTypePath.mkString("\n")) } val fieldValue = Invoke(inputObject, fieldName, dataTypeFor(fieldType)) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala index f8e1ebbc9fa02..df8f4b0610af2 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala @@ -792,7 +792,7 @@ class DatasetSuite extends QueryTest with SharedSQLContext { Seq(InvalidInJava(1)).toDS() } assert(e.getMessage.contains( - "`abstract` is a java reserved keyword and can not be used as field name")) + "`abstract` is a reserved keyword and cannot be used as field name")) } }