jackylee-ch commented on code in PR #47268:
URL: https://github.com/apache/spark/pull/47268#discussion_r1671551627


##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala:
##########
@@ -157,10 +157,15 @@ private[hive] case class HiveGenericUDF(
     val setValues = evals.zipWithIndex.map {
       case (eval, i) =>
         s"""
-           |if (${eval.isNull}) {
-           |  $refEvaluator.setArg($i, null);
-           |} else {
-           |  $refEvaluator.setArg($i, ${eval.value});
+           |try {
+           |  ${eval.code}
+           |  if (${eval.isNull}) {
+           |    $refEvaluator.setArg($i, null);
+           |  } else {
+           |    $refEvaluator.setArg($i, ${eval.value});
+           |  }

Review Comment:
   >> It seems that catch (...) is only related to ${eval.code}
   
   Yes, we should catch the exceptions from child's executions.
   
   >> Why is it Exception instead of Throwable?
   
   The Exception can catch most of throwable problems. It's ok to be change to 
Throwable.



##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -131,6 +131,13 @@ class HiveGenericUDFEvaluator(
   def setArg(index: Int, arg: Any): Unit =
     deferredObjects(index).asInstanceOf[DeferredObjectAdapter].set(arg)
 
+  def setFuncArg(index: Int, arg: () => Any): Unit =

Review Comment:
   It is a little ugly that we use inner class in codegen. If we use 
`setFuncArg`, the codegen for exceptions would be follow code.
   ```
           s"""
              |try {
              |  ${eval.code}
              |  if (${eval.isNull}) {
              |    $refEvaluator.setArg($i, null);
              |  } else {
              |    $refEvaluator.setArg($i, ${eval.value});
              |  }
              |} catch (Exception exp) {
              |  final exception = exp;
              |  $refEvaluator.setFundArg($i, new scala.Function0<Object>() {
              |    public Object apply() {
              |      throw exception;
              |    }
              |  });
              |}
              |""".stripMargin
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to