[SPARK-57902][SQL] Emit a ternary for 2-arg Coalesce with a non-nullable pure fallback

[gengliangwang]

What changes were proposed in this pull request?

The general Coalesce codegen evaluates its children inside a do { ... } while (false) loop and,
before that, promotes the result's null flag to a class-level mutable field via
ctx.addMutableState:

ev.isNull = JavaCode.isNullGlobal(ctx.addMutableState(CodeGenerator.JAVA_BOOLEAN, ev.isNull))

It has to be a field rather than a local boolean because the branches may be split out into a
separate coalesce_N(...) method (via splitExpressionsWithCurrentInputs), so the flag must be
visible across that method boundary. So every coalesce on the general path adds one
private boolean ...; field to the generated class, e.g. for coalesce(a, b):

private boolean coalesce_isNull_0;          // added to the generated class
...
coalesce_isNull_0 = true;
int coalesce_value_0 = -1;
do {
  if (!aIsNull) { coalesce_isNull_0 = false; coalesce_value_0 = aValue; continue; }
  if (!bIsNull) { coalesce_isNull_0 = false; coalesce_value_0 = bValue; continue; }
} while (false);

For coalesce(a, b) where b is non-nullable and its evaluation emits no code (a literal or an
already-evaluated value) -- e.g. the coalesce(sum, 0) in every SUM/AVG update -- the result can
never be null and b carries a pure Java expression, so the whole thing collapses to a single
ternary with no field and no loop (isNull becomes the compile-time constant
FalseLiteral):

int coalesce_value_0 = aIsNull ? 0 : aValue;

The empty-code requirement preserves lazy evaluation: there are no statements of b to hoist past
the null check. When the fast path does not apply, the general path reuses the ExprCode already
generated while probing (generating a child twice would duplicate codegen side effects on the
context, such as orphaned mutable state for stateful expressions and repeated reference
registrations).

Note on why this shrinks the compiled artifact and not just the source: the generated code is
compiled by Janino, which does no dead-code elimination or field pruning, so the removed
do/while scaffold and the private boolean field (which also occupies a constant-pool entry and
per-instance state) genuinely disappear from the generated class -- they are not something a
compiler would have optimized away. Spark also intentionally bounds the number of mutable states,
which is what the SPARK-22705 test guards.

Why are the changes needed?

This is part of the effort to reduce generated Java size in whole-stage codegen (parent
SPARK-56908). Smaller generated methods reduce
Janino compilation work, JIT work, and pressure against the JVM 64KB method limit. Measured on a
TPC-DS codegen dump (150 queries, 1,572 whole-stage-codegen subtrees): about -2% of total
generated source (109/150 queries smaller, 0 larger), and one global mutable field removed per
affected coalesce.

Does this PR introduce any user-facing change?

No. This only changes the shape of generated code; results are unchanged.

How was this patch tested?

NullExpressionsSuite -- added a test asserting the two-argument non-nullable-pure-fallback form
compiles to a ternary with no global mutable state and evaluates correctly on both the null and
non-null branches. The existing SPARK-22705 global-variable test is updated to use three children
so it continues to exercise the general do-while path (the two-argument form it previously used now
takes the new fast path).

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.8)

[uros-b]

Noting a minor test coverage gap: the new fast-path test uses only Literal inputs and StringType. The motivating hot path is coalesce(nullable_column, 0) where the first arg is a BoundReference (a distinct codegen path via currentVars lookup, not ExprCode.forNonNullValue) and the fallback is a primitive.

Adding a BoundReference-through-a-row case (both null and non-null branches, e.g. checkEvaluation(Coalesce(Seq(BoundReference(0, IntegerType, nullable = true), Literal(0))), 42, InternalRow(42))) would pin the actual motivating path.

[uros-b]

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.

[uros-b]

Thank you @gengliangwang, I left a few comments regarding unit tests coverage

[cloud-fan]

0 blocking, 0 non-blocking, 0 nits.
Clean, well-scoped codegen optimization; correct and consistent with the existing NaNvl/If ternary-with-FalseLiteral idiom.

No findings.

Verification

Traced the fast-path ternary a.isNull ? b.value : a.value (isNull=FalseLiteral) against the general do-while for every input dimension: a null / a non-null both agree; result nullability is correct since b non-nullable makes Coalesce.nullable false; the empty-b.code gate preserves lazy eval (no statements of b hoisted before a's null check) and keeps b a single pure expression, so nondeterminism/eval-count is unchanged. NullPropagation truncates a Coalesce at its first non-nullable child, so coalesce(nullable, non-nullable) is exactly the surviving shape this targets. Confirmed the test's inlinedMutableStates.isEmpty holds (a string Literal emits empty code via forNonNullValue; addReferenceObj appends to references, not inlinedMutableStates) and that checkEvaluation exercises the codegen path.

@uros-b's two test-coverage suggestions (BoundReference+primitive path, and a direct test for the probe-reuse branch) are both valid and stand on their own.

[gengliangwang]

Thanks @uros-b! Addressed both: added a coalesce(nullable BoundReference, primitive literal) test exercising the ternary fast path from a row read (null + non-null branches), and a direct test for the probe-reuse branch (2-arg, non-nullable fallback whose code is non-empty, so the probed ExprCodes are threaded into the general path). NullExpressionsSuite passes.

[LuciferYang]

import order issue

[gengliangwang]

Fixed — reordered so FunctionIdentifier precedes InternalRow.

[LuciferYang]

CodeGenerator.javaType(dataType) is computed in both the fast-path branch and the general path(line 137).

[gengliangwang]

Good catch — hoisted val resultType = CodeGenerator.javaType(dataType) to the top of doGenCode so it's computed once and shared by both the fast path and the general path.

[LuciferYang]

nit: children.head

[gengliangwang]

Done — switched to children.head.

[gengliangwang]

Merge Summary:

• merged into master c116a27
• merged into branch-4.x a8c1bce

Posted by merge_spark_pr.py