uros-b commented on code in PR #56971:
URL: https://github.com/apache/spark/pull/56971#discussion_r3524904325
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/NullExpressionsSuite.scala:
##########
@@ -207,10 +207,21 @@ class NullExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
test("SPARK-22705: Coalesce should use less global variables") {
val ctx = new CodegenContext()
- Coalesce(Seq(Literal("a"), Literal("b"))).genCode(ctx)
+ // Use three children so that the general do-while codegen path is
exercised: the two-argument
+ // form with a non-nullable pure fallback is now special-cased to a
ternary with no global
+ // state (covered by the test below).
+ Coalesce(Seq(Literal("a"), Literal("b"), Literal("c"))).genCode(ctx)
assert(ctx.inlinedMutableStates.size == 1)
}
+ test("Coalesce with a non-nullable pure fallback compiles to a ternary with
no global state") {
+ val ctx = new CodegenContext()
+ Coalesce(Seq(Literal.create(null, StringType), Literal("b"))).genCode(ctx)
+ assert(ctx.inlinedMutableStates.isEmpty)
+ checkEvaluation(Coalesce(Seq(Literal.create(null, StringType),
Literal("a"))), "a")
+ checkEvaluation(Coalesce(Seq(Literal("x"), Literal("y"))), "x")
+ }
Review Comment:
Also, the subtlest branch, which is the probe-reuse path (2-arg,
non-nullable b, but b.code NON-empty, so Some(Seq(first, second)) is threaded
into the general path), is only covered indirectly by end-to-end suites, not by
a direct unit test. Adding a direct regression test would be nice here too.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
