[GitHub] [spark] tomvanbussel commented on a change in pull request #34670: [SPARK-37388][SQL] Fix NPE in WidthBucket in WholeStageCodegenExec

[GitBox <gi...@apache.org>]

tomvanbussel commented on a change in pull request #34670:
URL: https://github.com/apache/spark/pull/34670#discussion_r753522763



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
##########
@@ -1666,9 +1677,14 @@ case class WidthBucket(
   }
 
   override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
-    defineCodeGen(ctx, ev, (input, min, max, numBucket) =>
-      "org.apache.spark.sql.catalyst.expressions.WidthBucket" +
-        s".computeBucketNumber($input, $min, $max, $numBucket)")
+    nullSafeCodeGen(ctx, ev, (input, min, max, numBucket) => {

Review comment:
       Yes, with `nullSafeCodeGen` we will be able to handle null inputs. In fact, `defineCodeGen` already used `nullSafeCodeGen`, so we were already able to handle null inputs before. The issue is that `computeBucketNumber()` can return null when none of the inputs are null. Before this change it would throw an NPE in that case because it would try to assign null to a `long`.




-- 
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-unsubscribe@spark.apache.org

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



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