-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-21567][SQL] Dataset should work with type alias #18813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,7 +62,7 @@ object ScalaReflection extends ScalaReflection { | |
| def dataTypeFor[T : TypeTag]: DataType = dataTypeFor(localTypeOf[T]) | ||
|
|
||
| private def dataTypeFor(tpe: `Type`): DataType = { | ||
| tpe match { | ||
| tpe.dealias match { | ||
| case t if t <:< definitions.IntTpe => IntegerType | ||
| case t if t <:< definitions.LongTpe => LongType | ||
| case t if t <:< definitions.DoubleTpe => DoubleType | ||
|
|
@@ -94,7 +94,7 @@ object ScalaReflection extends ScalaReflection { | |
| * JVM form instead of the Scala Array that handles auto boxing. | ||
| */ | ||
| private def arrayClassFor(tpe: `Type`): ObjectType = { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. arrayClassFor is called at many place. The typical calling pattern looks like: So instead of dealiasing when calling, we dealiase it here. |
||
| val cls = tpe match { | ||
| val cls = tpe.dealias match { | ||
| case t if t <:< definitions.IntTpe => classOf[Array[Int]] | ||
| case t if t <:< definitions.LongTpe => classOf[Array[Long]] | ||
| case t if t <:< definitions.DoubleTpe => classOf[Array[Double]] | ||
|
|
@@ -193,7 +193,7 @@ object ScalaReflection extends ScalaReflection { | |
| case _ => UpCast(expr, expected, walkedTypePath) | ||
| } | ||
|
|
||
| tpe match { | ||
| tpe.dealias match { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for |
||
| case t if !dataTypeFor(t).isInstanceOf[ObjectType] => getPath | ||
|
|
||
| case t if t <:< localTypeOf[Option[_]] => | ||
|
|
@@ -469,7 +469,7 @@ object ScalaReflection extends ScalaReflection { | |
| } | ||
| } | ||
|
|
||
| tpe match { | ||
| tpe.dealias match { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For |
||
| case _ if !inputObject.dataType.isInstanceOf[ObjectType] => inputObject | ||
|
|
||
| case t if t <:< localTypeOf[Option[_]] => | ||
|
|
@@ -643,7 +643,7 @@ object ScalaReflection extends ScalaReflection { | |
| * we also treat [[DefinedByConstructorParams]] as product type. | ||
| */ | ||
| def optionOfProductType(tpe: `Type`): Boolean = { | ||
| tpe match { | ||
| tpe.dealias match { | ||
| case t if t <:< localTypeOf[Option[_]] => | ||
| val TypeRef(_, _, Seq(optType)) = t | ||
| definedByConstructorParams(optType) | ||
|
|
@@ -690,7 +690,7 @@ object ScalaReflection extends ScalaReflection { | |
| /* | ||
| * Retrieves the runtime class corresponding to the provided type. | ||
| */ | ||
| def getClassFromType(tpe: Type): Class[_] = mirror.runtimeClass(tpe.typeSymbol.asClass) | ||
| def getClassFromType(tpe: Type): Class[_] = mirror.runtimeClass(tpe.dealias.typeSymbol.asClass) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be saved from dealiasing. I'll remove it. |
||
|
|
||
| case class Schema(dataType: DataType, nullable: Boolean) | ||
|
|
||
|
|
@@ -705,7 +705,7 @@ object ScalaReflection extends ScalaReflection { | |
|
|
||
| /** Returns a catalyst DataType and its nullability for the given Scala Type using reflection. */ | ||
| def schemaFor(tpe: `Type`): Schema = { | ||
| tpe match { | ||
| tpe.dealias match { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can't be saved from dealiasing. |
||
| case t if t.typeSymbol.annotations.exists(_.tree.tpe =:= typeOf[SQLUserDefinedType]) => | ||
| val udt = getClassFromType(t).getAnnotation(classOf[SQLUserDefinedType]).udt().newInstance() | ||
| Schema(udt, nullable = true) | ||
|
|
@@ -775,7 +775,7 @@ object ScalaReflection extends ScalaReflection { | |
| * Whether the fields of the given type is defined entirely by its constructor parameters. | ||
| */ | ||
| def definedByConstructorParams(tpe: Type): Boolean = { | ||
| tpe <:< localTypeOf[Product] || tpe <:< localTypeOf[DefinedByConstructorParams] | ||
| tpe.dealias <:< localTypeOf[Product] || tpe.dealias <:< localTypeOf[DefinedByConstructorParams] | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be saved from dealiasing. I'll remove it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh. no. |
||
| } | ||
|
|
||
| private val javaKeywords = Set("abstract", "assert", "boolean", "break", "byte", "case", "catch", | ||
|
|
@@ -829,7 +829,7 @@ trait ScalaReflection { | |
| * synthetic classes, emulating behaviour in Java bytecode. | ||
| */ | ||
| def getClassNameFromType(tpe: `Type`): String = { | ||
| tpe.erasure.typeSymbol.asClass.fullName | ||
| tpe.dealias.erasure.typeSymbol.asClass.fullName | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed. |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -848,9 +848,10 @@ trait ScalaReflection { | |
| * support inner class. | ||
| */ | ||
| def getConstructorParameters(tpe: Type): Seq[(String, Type)] = { | ||
| val formalTypeArgs = tpe.typeSymbol.asClass.typeParams | ||
| val TypeRef(_, _, actualTypeArgs) = tpe | ||
| val params = constructParams(tpe) | ||
| val dealiasedTpe = tpe.dealias | ||
| val formalTypeArgs = dealiasedTpe.typeSymbol.asClass.typeParams | ||
| val TypeRef(_, _, actualTypeArgs) = dealiasedTpe | ||
| val params = constructParams(dealiasedTpe) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed. |
||
| // if there are type variables to fill in, do the substitution (SomeClass[T] -> SomeClass[Int]) | ||
| if (actualTypeArgs.nonEmpty) { | ||
| params.map { p => | ||
|
|
@@ -864,7 +865,7 @@ trait ScalaReflection { | |
| } | ||
|
|
||
| protected def constructParams(tpe: Type): Seq[Symbol] = { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is called at different points. So it's needed too. |
||
| val constructorSymbol = tpe.member(termNames.CONSTRUCTOR) | ||
| val constructorSymbol = tpe.dealias.member(termNames.CONSTRUCTOR) | ||
| val params = if (constructorSymbol.isMethod) { | ||
| constructorSymbol.asMethod.paramLists | ||
| } else { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,16 @@ import org.apache.spark.sql.types._ | |
| case class TestDataPoint(x: Int, y: Double, s: String, t: TestDataPoint2) | ||
| case class TestDataPoint2(x: Int, s: String) | ||
|
|
||
| object TestForTypeAlias { | ||
| type TwoInt = (Int, Int) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's also test nested type alias
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry what the nested type alias means?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. like type TwoIntSeq = Seq[TwoInt]?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. Added another test for this case. |
||
| type ThreeInt = (TwoInt, Int) | ||
| type SeqOfTwoInt = Seq[TwoInt] | ||
|
|
||
| def tupleTypeAlias: TwoInt = (1, 1) | ||
| def nestedTupleTypeAlias: ThreeInt = ((1, 1), 2) | ||
| def seqOfTupleTypeAlias: SeqOfTwoInt = Seq((1, 1), (2, 2)) | ||
| } | ||
|
|
||
| class DatasetSuite extends QueryTest with SharedSQLContext { | ||
| import testImplicits._ | ||
|
|
||
|
|
@@ -1317,6 +1327,20 @@ class DatasetSuite extends QueryTest with SharedSQLContext { | |
| checkAnswer(df.orderBy($"id"), expected) | ||
| checkAnswer(df.orderBy('id), expected) | ||
| } | ||
|
|
||
| test("SPARK-21567: Dataset should work with type alias") { | ||
| checkDataset( | ||
| Seq(1).toDS().map(_ => ("", TestForTypeAlias.tupleTypeAlias)), | ||
| ("", (1, 1))) | ||
|
|
||
| checkDataset( | ||
| Seq(1).toDS().map(_ => ("", TestForTypeAlias.nestedTupleTypeAlias)), | ||
| ("", ((1, 1), 2))) | ||
|
|
||
| checkDataset( | ||
| Seq(1).toDS().map(_ => ("", TestForTypeAlias.seqOfTupleTypeAlias)), | ||
| ("", Seq((1, 1), (2, 2)))) | ||
| } | ||
| } | ||
|
|
||
| case class WithImmutableMap(id: String, map_test: scala.collection.immutable.Map[Long, String]) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dataTypeFor can be called like this at many places:
So we need to dealias it too.