-
Notifications
You must be signed in to change notification settings - Fork 29k
[Only Test] Make HiveGenericUDF's DeferredObject lazy #47193
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
Conversation
|
|
||
| def setArg(index: Int, arg: Any): Unit = | ||
| deferredObjects(index).asInstanceOf[DeferredObjectAdapter].set(arg) | ||
| deferredObjects(index).asInstanceOf[DeferredObjectAdapter].set(() => arg) |
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.
cc @jackylee-ch Can you test this fix?
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.
cc @jackylee-ch Can you test this fix?
I have some change in my local. Like @cloud-fan said, we should pass function to setArg to make it lazy. I have passed the failed case with none codegen case, and I'm still trying to fix it with codegen.
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.
@jackylee-ch Can you help test the updated version again?
Thanks.
|
|
||
| override def returnType: DataType = inspectorToDataType(returnInspector) | ||
|
|
||
| def setArg(index: Int, arg: Any): Unit = |
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.
I think we should make it lazy at the very beginning, arg here is already materialized.
| | return $finalEval; | ||
| | } | ||
| | }; | ||
| |$refEvaluator.setFuncArg($i, $funcArg); |
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.
This won't work as we should mv the eval.code in apply, so that we can lazy run child functions. Just like the following code.
|$refEvaluator.setArg($i, new scala.runtime.AbstractFunction0<Object>() {
| public Object apply() {
| ${eval.code}
| return ${eval.value};
| }
|});
""".stripMargin
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?