-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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
[SPARK-21837][SQL][TESTS] UserDefinedTypeSuite Local UDTs not actually testing what it intends #19053
Conversation
## What changes were proposed in this pull request? Enable ScalaReflection and User Defined Types for plain Scala classes. This involves the move of `schemaFor` from `ScalaReflection` trait (which is Runtime and Compile time (macros) reflection) to the `ScalaReflection` object (runtime reflection only) as I believe this code wouldn't work at compile time anyway as it manipulates `Class`'s that are not compiled yet. ## How was this patch tested? Unit test Author: Joan <joan@goyeau.com> Closes #12149 from joan38/SPARK-13929-Scala-reflection.
val df = Seq((1, vec)).toDF("int", "vec") | ||
assert(vec === df.collect()(0).getAs[UDT.MyDenseVector](1)) | ||
assert(vec === df.take(1)(0).getAs[UDT.MyDenseVector](1)) | ||
assert(vec === df.limit(1).groupBy('int).agg(first('vec)).collect()(0) |
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.
Note that this and the next line fails with the assert added, if it is left dereferencing col 0 at the end instead of 1.
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.
Since the last two lines are not testing collect
, we can rewrite them to
checkAnswer(df.limit(1).groupBy('int).agg(first('vec)), Row(1, vec))
checkAnswer(df.orderBy('int).limit(1).groupBy('int).agg(first('vec)), Row(1, vec))
Test build #81130 has finished for PR 19053 at commit
|
UDF? |
Oops, I meant UDT. Just referring to the tests name in the code. |
LGTM except one comment. |
Test build #3903 has finished for PR 19053 at commit
|
LGTM |
Test build #81137 has finished for PR 19053 at commit
|
Thanks! Merging to master. |
What changes were proposed in this pull request?
Adjust Local UDTs test to assert about results, and fix index of vector column. See JIRA for details.
How was this patch tested?
Existing tests.