[SPARK-35384][SQL][FOLLOWUP] Move HashMap.get out of InvokeLike.invoke
#32532
Conversation
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
| } else { | ||
| ret | ||
| } | ||
| boxingFn(ret) |
There was a problem hiding this comment.
This could speed up the performance a bit.
Are you going to attach the new benchmark result?
There was a problem hiding this comment.
Thanks. Yes I'll attach benchmark results for this later.
|
Test build #138499 has finished for PR 32532 at commit
|
| } else { | ||
| ret | ||
| } | ||
| boxingFn(ret) |
There was a problem hiding this comment.
I'm probably missing something dumb here but dataType is an argument to this method - how can we have one function for it? I don't see where dataType is available to the trait
There was a problem hiding this comment.
It's a bit misleading by looking at it, but in the call sites of invoke we currently always pass Expression.dataType (which is overridden by Invoke and StaticInvoke) as its argument, so the parameter is actually unnecessary. The same goes to arguments. To avoid confusion I think we can just remove these two parameters.
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #138527 has finished for PR 32532 at commit
|
|
@dongjoon-hyun @srowen let me know if you have more comments - also cc @cloud-fan @viirya |
### What changes were proposed in this pull request? Fix Scala doc for removed parameters for `InvokeLike.invoke`. ### Why are the changes needed? #32532 forgot to update the Scala doc after removing 2 parameters for `InvokeLike.invoke`. This fixes it. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? N/A Closes #32827 from sunchao/SPARK-35384-followup. Authored-by: Chao Sun <sunchao@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
5 participants
What changes were proposed in this pull request?
Move hash map lookup operation out of
InvokeLike.invokesince it doesn't depend on the input.Why are the changes needed?
We shouldn't need to look up the hash map for every input row evaluated by
InvokeLike.invokesince it doesn't depend on input. This could speed up the performance a bit.Does this PR introduce any user-facing change?
No
How was this patch tested?
Existing tests.