[SPARK-23585][SQL] Add interpreted execution to UnwrapOption #20736
Conversation
| override def eval(input: InternalRow): Any = | ||
| throw new UnsupportedOperationException("Only code-generated evaluation is supported") | ||
| override def eval(input: InternalRow): Any = { | ||
| val inputObject = child.eval(input) |
There was a problem hiding this comment.
NIT: you can also use pattern matching here. That saves you some typing:
child.eval(input) match {
case Some(value) => value
case _ => null
}There was a problem hiding this comment.
thanks for your comment. I haven't used pattern matching because it is a bit slower than using if...else and I'd prefer the fastest way here. What do you think?
There was a problem hiding this comment.
I am fine either way. This is going to be slow anyway. Let's leave as it is.
| mapEncoder.serializer.head, mapExpected, mapInputRow) | ||
| } | ||
|
|
||
| test("SPARK-23586: UnwrapOption should support interpreted execution") { |
There was a problem hiding this comment.
I just found out that there are literally no unit tests for this expression :(...
Can you use checkEvaluation? This will make sure both code gen and interpreted mode are tested?
|
@mgaido91 you picked this up phenomenally fast! It looks pretty good, let two minor comments. |
|
@hvanhovell thanks for your comments, let's say this is a good opportunity to improve test coverage too for it |
|
LGTM - pending jenkins |
mgaido91
changed the title
|
Test build #87957 has finished for PR 20736 at commit
|
|
Test build #87961 has finished for PR 20736 at commit
|
|
Test build #87963 has finished for PR 20736 at commit
|
|
Merging to master. Thanks! |
## What changes were proposed in this pull request? The PR adds interpreted execution to UnwrapOption. ## How was this patch tested? added UT Author: Marco Gaido <[email protected]> Closes apache#20736 from mgaido91/SPARK-23586.
3 participants
What changes were proposed in this pull request?
The PR adds interpreted execution to UnwrapOption.
How was this patch tested?
added UT