Skip to content

Commit

Permalink
[SPARK-25690][SQL] Analyzer rule HandleNullInputsForUDF does not stab…
Browse files Browse the repository at this point in the history
…ilize and can be applied infinitely

## What changes were proposed in this pull request?

The HandleNullInputsForUDF rule can generate new If node infinitely, thus causing problems like match of SQL cache missed.
This was fixed in SPARK-24891 and was then broken by SPARK-25044.
The unit test in `AnalysisSuite` added in SPARK-24891 should have failed but didn't because it wasn't properly updated after the `ScalaUDF` constructor signature change. So this PR also updates the test accordingly based on the new `ScalaUDF` constructor.

## How was this patch tested?

Updated the original UT. This should be justified as the original UT became invalid after SPARK-25044.

Closes #22701 from maryannxue/spark-25690.

Authored-by: maryannxue <[email protected]>
Signed-off-by: gatorsmile <[email protected]>
(cherry picked from commit 3685130)
Signed-off-by: gatorsmile <[email protected]>
  • Loading branch information
maryannxue authored and gatorsmile committed Oct 12, 2018
1 parent e80ab13 commit 1961f8e
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2164,8 +2164,10 @@ class Analyzer(

// TODO: skip null handling for not-nullable primitive inputs after we can completely
// trust the `nullable` information.
val needsNullCheck = (nullable: Boolean, expr: Expression) =>
nullable && !expr.isInstanceOf[KnownNotNull]
val inputsNullCheck = nullableTypes.zip(inputs)
.filter { case (nullable, _) => !nullable }
.filter { case (nullableType, expr) => needsNullCheck(!nullableType, 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,8 @@ class AnalysisSuite extends AnalysisTest with Matchers {
test("SPARK-24891 Fix HandleNullInputsForUDF rule") {
val a = testRelation.output(0)
val func = (x: Int, y: Int) => x + y
val udf1 = ScalaUDF(func, IntegerType, a :: a :: Nil)
val udf2 = ScalaUDF(func, IntegerType, a :: udf1 :: Nil)
val udf1 = ScalaUDF(func, IntegerType, a :: a :: Nil, nullableTypes = false :: false :: Nil)
val udf2 = ScalaUDF(func, IntegerType, a :: udf1 :: Nil, nullableTypes = false :: false :: Nil)
val plan = Project(Alias(udf2, "")() :: Nil, testRelation)
comparePlans(plan.analyze, plan.analyze.analyze)
}
Expand Down

0 comments on commit 1961f8e

Please sign in to comment.