Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22259#discussion_r213701008
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -2149,28 +2149,34 @@ class Analyzer(
case p => p transformExpressionsUp {
- case udf @ ScalaUDF(func, _, inputs, _, _, _, _) =>
- val parameterTypes = ScalaReflection.getParameterTypes(func)
- assert(parameterTypes.length == inputs.length)
-
- // TODO: skip null handling for not-nullable primitive inputs
after we can completely
- // trust the `nullable` information.
- // (cls, expr) => cls.isPrimitive && expr.nullable
- val needsNullCheck = (cls: Class[_], expr: Expression) =>
- cls.isPrimitive && !expr.isInstanceOf[KnownNotNull]
- val inputsNullCheck = parameterTypes.zip(inputs)
- .filter { case (cls, expr) => needsNullCheck(cls, expr) }
- .map { case (_, expr) => IsNull(expr) }
- .reduceLeftOption[Expression]((e1, e2) => Or(e1, e2))
- // Once we add an `If` check above the udf, it is safe to mark
those checked inputs
- // as not nullable (i.e., wrap them with `KnownNotNull`),
because the null-returning
- // branch of `If` will be called if any of these checked inputs
is null. Thus we can
- // prevent this rule from being applied repeatedly.
- val newInputs = parameterTypes.zip(inputs).map{ case (cls, expr)
=>
- if (needsNullCheck(cls, expr)) KnownNotNull(expr) else expr }
- inputsNullCheck
- .map(If(_, Literal.create(null, udf.dataType),
udf.copy(children = newInputs)))
- .getOrElse(udf)
+ case udf@ScalaUDF(func, _, inputs, _, _, _, _, nullableTypes) =>
--- End diff --
Oh right. Yeah I didn't mean to change that. It's minor enough to leave I
think. (or else standardize across the code)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
