[SPARK-11750][SQL] revert SPARK-11727 and code clean up #9726
Conversation
| @@ -718,39 +630,4 @@ trait ScalaReflection { | |||
| throw new UnsupportedOperationException(s"Schema for type $other is not supported") | |||
| } | |||
| } | |||
|
|
|||
| def typeOfObject: PartialFunction[Any, DataType] = { | |||
There was a problem hiding this comment.
This method is only used in tests, so I think we should remove it and its related tests.
|
cc @marmbrus |
|
Test build #45980 has finished for PR 9726 at commit
|
| val universe: scala.reflect.api.Universe | ||
|
|
||
| /** The mirror used to access types in the universe */ | ||
| def mirror: universe.Mirror |
There was a problem hiding this comment.
This level of indirection is on purpose. It lets us use the same reflection code both at runtime and in macros. Even if we aren't using that right now, we shouldn't undo the work that was done to make this possible.
|
Test build #46070 has finished for PR 9726 at commit
|
|
retest this please. |
|
Test build #46083 has finished for PR 9726 at commit
|
| @@ -736,39 +645,4 @@ trait ScalaReflection { | |||
| assert(methods.length == 1) | |||
| methods.head.getParameterTypes | |||
| } | |||
|
|
|||
| def typeOfObject: PartialFunction[Any, DataType] = { | |||
There was a problem hiding this comment.
This method is only used in tests, so I think we should remove it and its related tests.
There was a problem hiding this comment.
Probably fine in this case, but this isn't related to the undoing of the refactoring and mixing concerns makes the PR much harder to review.
|
Test build #46314 has finished for PR 9726 at commit
|
| // We convert the not-serializable TypeTag into StructType and ClassTag. | ||
| val mirror = typeTag[T].mirror | ||
| val cls = mirror.runtimeClass(typeTag[T].tpe) | ||
| val flat = !classOf[Product].isAssignableFrom(cls) |
There was a problem hiding this comment.
This isn't going to work for java. We should just leave this as it was.
There was a problem hiding this comment.
But we need a way to detect flatness automatically, sometimes we only have the type T and nothing else, like when we wanna use encoders for ScalaUDF. How about flat = !ScalaReflection.schemaFor[T].isInstanceOf[StructType])?
After some experiment, I found it's not convenient to have separate encoder builders: `FlatEncoder` and `ProductEncoder`. For example, when create encoders for `ScalaUDF`, we have no idea if the type `T` is flat or not. So I revert the splitting change in #9693, while still keeping the bug fixes and tests. Author: Wenchen Fan <wenchen@databricks.com> Closes #9726 from cloud-fan/follow. (cherry picked from commit 47d1c23) Signed-off-by: Michael Armbrust <michael@databricks.com>
After some experiment, I found it's not convenient to have separate encoder builders:
FlatEncoderandProductEncoder. For example, when create encoders forScalaUDF, we have no idea if the typeTis flat or not. So I revert the splitting change in #9693, while still keeping the bug fixes and tests.