Uh oh!
There was an error while loading. Please reload this page.
[SPARK-12293][SQL] Support UnsafeRow in LocalTableScan - #10283
Conversation
SparkQA
commented
Dec 13, 2015
Test build #47628 has finished for PR 10283 at commit
|
SparkQA
commented
Dec 14, 2015
Test build #47634 has finished for PR 10283 at commit
|
There was a problem hiding this comment.
The return type of UnsafeProjection.apply is UnsafeRow already, looks like we don't need the asInstanceOf here?
viirya
commented
Dec 14, 2015
I think the commits to fix these test errors are worth having their PR. I will create them later. |
There was a problem hiding this comment.
Normally Decimal should only be used inside spark SQL as the internal representation of decimal type, and we don't need to catch it here. Do we break it in tests?
There was a problem hiding this comment.
Yes. constructorFor will call dataTypeFor to determine if a type is ObjectType or not. If there is not case for Decimal, it will be recognized as ObjectType and causes bug.
SparkQA
commented
Dec 14, 2015
Test build #47651 has finished for PR 10283 at commit
|
There was a problem hiding this comment.
If the unwrapped is null, the Invoke will return null, see https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects.scala#L172-L175
I think we don't need this extra If, did I miss something here?
There was a problem hiding this comment.
I think you are right. I add this to make sure it is not possibly creating wrong values when I debug. I will remove these If.
SparkQA
commented
Dec 16, 2015
Test build #47829 has finished for PR 10283 at commit
|
SparkQA
commented
Dec 17, 2015
Test build #47888 has finished for PR 10283 at commit
|
viirya
commented
Dec 17, 2015
retest this please. |
SparkQA
commented
Dec 17, 2015
Test build #47911 has finished for PR 10283 at commit
|
viirya
commented
Dec 17, 2015
retest this please. |
SparkQA
commented
Dec 17, 2015
Test build #47920 has finished for PR 10283 at commit
|
There was a problem hiding this comment.
we can use encoderFor[T] here
viirya
commented
Dec 19, 2015
SQLUserDefinedType support for encoder is proposed in another pr #10390. |
viirya
commented
Dec 21, 2015
@cloud-fan I think I have addressed all your comments. These bugs found in implementing UnsafeRow support in LocalTableScan are submitted as other PRs with their tests, so you can review them better. |
SparkQA
commented
Dec 21, 2015
Test build #48094 has finished for PR 10283 at commit
|
viirya
commented
Dec 21, 2015
retest this please. |
JIRA: https://issues.apache.org/jira/browse/SPARK-12293
Make LocalTableScan support UnsafeRow.