[SPARK-57180][SQL] Skip statically-dead setNullAt branch in GenerateSafeProjection for non-nullable fields#56231
Open
gengliangwang wants to merge 1 commit into
Conversation
…afeProjection 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.
### 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)
Co-authored-by: Isaac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
GenerateSafeProjectionemitsif (isNull) { mutableRow.setNullAt(i); } else { convert; setColumn; }for every projected field. When the field expression is statically non-nullable,isNullisFalseLiteral, so thesetNullAtbranch is dead and only theelseever runs. This patch detects that case (evaluationCode.isNull == FalseLiteral-- the idiom already used byGenerateUnsafeProjection) and emits just the conversion +setColumn. The nullable path is unchanged. (GenerateMutableProjectionalready branches one.nullable; this bringsGenerateSafeProjectionin line.)Why are the changes needed?
Part of SPARK-56908 (umbrella). Removes a dead branch (and the
setNullAtcall) 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 fieldstoGeneratedProjectionSuite, projecting non-nullable int/string/struct/array fields through an unsafe -> safe round trip and asserting the values.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)