[SPARK-23584][SQL] NewInstance should support interpreted execution #20778
Conversation
|
Test build #88108 has finished for PR 20778 at commit
|
| @@ -138,4 +154,40 @@ class ObjectExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { | |||
| checkEvaluation(decodeUsingSerializer, null, InternalRow.fromSeq(Seq(null))) | |||
| } | |||
| } | |||
|
|
|||
| // This is an alternative version of `checkEvaluation` to compare results | |||
There was a problem hiding this comment.
This function was copy from #20757, so I'll remove later.
| Seq(expr.dataType.asInstanceOf[ObjectType].cls)) | ||
| } | ||
| val findConstructor = (types: Seq[Seq[Class[_]]]) => { | ||
| val constructorOption = cls.getConstructors.find { c => |
There was a problem hiding this comment.
Can we directly call cls.getConstructor with required Class to get it?
There was a problem hiding this comment.
I tried that way first, but I gave it up because the number of cls.getConstructor calls might blow up if there are many JVM type combinations in constructor arguments (e.g., constructor(java.Integer or scala.Int, java.Double or scala.Double, ....)).
There was a problem hiding this comment.
How does this compare to JVM resolution rules? The code generated version uses those.
There was a problem hiding this comment.
I checked the resolution rule in Class.getConstructor and I found it only supported the exact matching for parameter types (so, we couldn't handle class inheritances correctly). Then, in the latest fix, I tried to use getMatchingAccessibleConstructor in commons.lang3. It seems useful to find an acceptable constructor in a class. How about this? @hvanhovell @viirya
|
Test build #88920 has finished for PR 20778 at commit
|
|
Test build #89237 has finished for PR 20778 at commit
|
|
retest this please |
|
Test build #89240 has finished for PR 20778 at commit
|
|
Test build #89546 has finished for PR 20778 at commit
|
|
retest this please |
|
Test build #89565 has finished for PR 20778 at commit
|
What changes were proposed in this pull request?
This pr supported interpreted mode for
NewInstance.How was this patch tested?
Added tests in
ObjectExpressionsSuite.