You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "gengliangwang (via GitHub)" <gi...@apache.org> on 2026/06/01 22:51:40 UTC
[PR] [SPARK-57202][SQL] Skip dead null checks in In expression codegen for non-null list elements [spark]
gengliangwang opened a new pull request, #56258:
URL: https://github.com/apache/spark/pull/56258
### What changes were proposed in this pull request?
`In`'s whole-stage codegen emits, for each list element `x`:
```java
if (x.isNull) {
inTmpResult = -1; // HAS_NULL
} else if (value == x) {
inTmpResult = 1; // MATCHED
continue;
}
```
IN lists are usually constant literals, so `x.isNull` is the literal `false` and the `HAS_NULL` branch is dead (`if (false) { inTmpResult = -1; } else if ...`). This PR emits only the equality check when `x.isNull == FalseLiteral`. Symmetrically, when an element is statically null (`x.isNull == TrueLiteral`, e.g. `IN (NULL, ...)`), only the `HAS_NULL` assignment is emitted and the dead equality check is dropped.
### Why are the changes needed?
Sub-task of SPARK-56908 (reduce generated Java size in whole-stage codegen). Dumping the TPC-DS whole-stage codegen shows ~348 dead `if (false) { inTmpResult = -1; } else if (...)` blocks from `In` over literal lists. Emitting only the live branch removes the dead conditional from the generated code.
### Does this PR introduce _any_ user-facing change?
No. The generated code is smaller but evaluates IN with identical (including null) semantics: a statically non-null element can never set `HAS_NULL`, and a statically null element can never match, so dropping those dead branches is equivalent.
### How was this patch tested?
Behavior-preserving change covered by `PredicateSuite` (70 tests, including `In` with null list elements and a null left-hand value), all pass. Additionally verified by re-dumping the TPC-DS whole-stage codegen: the ~348 dead `if (false)` blocks in `In` are gone and every generated subtree still compiles.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57202][SQL] Skip dead null checks in In expression codegen for non-null list elements [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #56258:
URL: https://github.com/apache/spark/pull/56258#issuecomment-4616587301
On reflection, this PR doesn't meet the bar set out in the [SPARK-56908](https://issues.apache.org/jira/browse/SPARK-56908) scope guidance, so I'm withdrawing it.
It's a source-only dead-branch elimination: for a non-null literal list element, `if (x.isNull) { ... } else if (...)` becomes `if (false) { ... } else if (...)`, and Janino constant-folds `if (false)` — so the bytecode is unchanged. The dead branch sets an `iconst` (`HAS_NULL = -1`), not a constant-pool entry, and the literal it reads is already used in the live equality branch, so nothing is removed from `references[]` / the constant pool. The pattern is frequent (~348× across the TPC-DS whole-stage codegen), but per the measured data even ×358–445 source-only patterns saved compile time below the ~0.7% run-to-run noise floor.
So it adds codegen-logic branching for a benefit below measurement noise, with no bytecode / constant-pool change — exactly the case the scope guidance says to avoid. Closing.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57202][SQL] Skip dead null checks in In expression codegen for non-null list elements [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #56258: [SPARK-57202][SQL] Skip dead null checks in In expression codegen for non-null list elements
URL: https://github.com/apache/spark/pull/56258
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57202][SQL] Skip dead null checks in In expression codegen for non-null list elements [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #56258:
URL: https://github.com/apache/spark/pull/56258#issuecomment-4616612331
cc @LuciferYang @dongjoon-hyun FYI I closed my own PR since it idoesn't meet the bar set out in the the [SPARK-56908](https://issues.apache.org/jira/browse/SPARK-56908) scope guidance.
I think we can still keep the open PRs for decimal and CheckOverflow.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org