Skip to content

Commit 2d83237

Browse files
committed
[SPARK-57180][SQL] Skip statically-dead setNullAt branch in GenerateSafeProjection for non-nullable fields
### What changes were proposed in this pull request? `GenerateSafeProjection` emits `if (isNull) { mutableRow.setNullAt(i); } else { convert; setColumn; }` for every projected field. When the field expression is statically non-nullable, `isNull` is `FalseLiteral`, so the `setNullAt` branch is dead and only the `else` ever runs. This patch detects that case (`evaluationCode.isNull == FalseLiteral` -- the idiom already used by `GenerateUnsafeProjection`) and emits just the conversion + `setColumn`. The nullable path is unchanged. (`GenerateMutableProjection` already branches on `e.nullable`; this brings `GenerateSafeProjection` in line.) ### Why are the changes needed? Part of SPARK-56908 (umbrella). Removes a dead branch (and the `setNullAt` call) per non-nullable field from the generated safe projection, shrinking the emitted Java for wide non-nullable schemas (helping with the JVM 64KB method / constant-pool limits, Janino compile time, and JIT work). ### Does this PR introduce _any_ user-facing change? No. The compiled behavior is identical; only the emitted Java source text changes. ### How was this patch tested? Added `SPARK-57180: SafeProjection over statically non-nullable fields` to `GeneratedProjectionSuite`, projecting non-nullable int/string/struct/array fields through an unsafe -> safe round trip and asserting the values. ``` build/sbt "catalyst/testOnly *GeneratedProjectionSuite" # 10/10 ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #56231 from gengliangwang/spark-safeprojection-deadnull-codegen. Authored-by: Gengliang Wang <gengliang@apache.org> Signed-off-by: Gengliang Wang <gengliang@apache.org>
1 parent 6b84c98 commit 2d83237

2 files changed

Lines changed: 39 additions & 3 deletions

File tree

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateSafeProjection.scala

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,15 +155,24 @@ object GenerateSafeProjection extends CodeGenerator[Seq[Expression], Projection]
155155
case (e, i) =>
156156
val evaluationCode = e.genCode(ctx)
157157
val converter = convertToSafe(ctx, evaluationCode.value, e.dataType)
158-
evaluationCode.code.toString +
158+
val setValue =
159+
s"""
160+
${converter.code}
161+
${CodeGenerator.setColumn("mutableRow", e.dataType, i, converter.value)};
162+
"""
163+
val writeField = if (evaluationCode.isNull == FalseLiteral) {
164+
// The expression is statically non-nullable, so the setNullAt branch is dead.
165+
setValue
166+
} else {
159167
s"""
160168
if (${evaluationCode.isNull}) {
161169
mutableRow.setNullAt($i);
162170
} else {
163-
${converter.code}
164-
${CodeGenerator.setColumn("mutableRow", e.dataType, i, converter.value)};
171+
$setValue
165172
}
166173
"""
174+
}
175+
evaluationCode.code.toString + writeField
167176
}
168177
val allExpressions = ctx.splitExpressionsWithCurrentInputs(expressionCodes)
169178

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/GeneratedProjectionSuite.scala

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,33 @@ class GeneratedProjectionSuite extends SparkFunSuite with ExpressionEvalHelper {
211211
assert(row.getStruct(0, 1).getString(0).toString == "a")
212212
}
213213

214+
test("SPARK-57180: SafeProjection over statically non-nullable fields") {
215+
// Non-nullable fields take the dead-branch-free path in GenerateSafeProjection (no
216+
// setNullAt). Cover atomic, struct and array types and verify the projected values.
217+
val fields = Array[DataType](
218+
IntegerType,
219+
StringType,
220+
new StructType().add("i", IntegerType, nullable = false),
221+
ArrayType(IntegerType, containsNull = false))
222+
val refs = fields.zipWithIndex.map { case (dt, i) =>
223+
BoundReference(i, dt, nullable = false)
224+
}.toImmutableArraySeq
225+
226+
val safeProj = GenerateSafeProjection.generate(refs)
227+
val unsafeProj = GenerateUnsafeProjection.generate(refs)
228+
val input = InternalRow(
229+
1,
230+
UTF8String.fromString("a"),
231+
InternalRow(2),
232+
new GenericArrayData(Array(3, 4)))
233+
val row = safeProj.apply(unsafeProj.apply(input))
234+
235+
assert(row.getInt(0) == 1)
236+
assert(row.getUTF8String(1).toString == "a")
237+
assert(row.getStruct(2, 1).getInt(0) == 2)
238+
assert(row.getArray(3).toIntArray().sameElements(Array(3, 4)))
239+
}
240+
214241
test("SPARK-22699: GenerateSafeProjection should not use global variables for struct") {
215242
val safeProj = GenerateSafeProjection.generate(
216243
Seq(BoundReference(0, new StructType().add("i", IntegerType), true)))

0 commit comments

Comments
 (0)