david-mollitor-db opened a new pull request, #58828: URL: https://github.com/apache/spark/pull/58828
### What changes were proposed in this pull request? `InSet` backs `IN` predicates with many elements (and `x = a OR x = b OR ...` chains once `OptimizeIn` folds them). In whole-stage-generated code, `InSet.genCodeWithSet` evaluates membership as `set.contains(value)` over a Scala `Set[Any]`; because `contains` takes `Object`, a primitive subject is autoboxed on every row and the set stores boxed elements. This PR adds a boxing-free path for integral and date/time types whose Catalyst internal representation is a primitive `int` or `long` (`Long`, `Timestamp`, `TimestampNTZ`, `Int`, `Date`, `Short`, `Byte`): a sorted primitive `long[]`/`int[]` is built once on the driver and probed in generated code with `java.util.Arrays.binarySearch`, a primitive overload that performs no autoboxing and no allocation. `Byte`/`Short` are widened to `int`. The new path is gated by an internal flag `spark.sql.optimizer.useSpecializedSetForInSet.enabled` (default `true`) and affects only whole-stage codegen: - the `switch` path (`Byte`/`Short`/`Int`/`Date` up to `spark.sql.optimizer.inSetSwitchThreshold`) is unchanged; - the interpreted `eval` path already receives a boxed value, so it is unchanged; - the `InSet` node itself is unchanged, so there is no plan-shape or golden-file change. `Float`/`Double` are intentionally excluded (their `NaN` and `-0.0`/`+0.0` SQL-equality semantics need the special handling the current path provides), as are `String`/`Decimal`/binary/complex types (not primitive; `UTF8String` is already an object, so there is no boxing to remove). ### Why are the changes needed? The JVM `switch` used by the fast path only supports `int` labels, so `Long`/`Timestamp`/`TimestampNTZ` always take the boxed `Set` path at every size, and large `Int`/`Date` sets (above the switch threshold) do too. Every row then pays a `Long.valueOf`/`Integer.valueOf` allocation for the `contains(Object)` call. Microbenchmark over a `Long` column, 10M rows, non-matching values (worst case — every row evaluates the full predicate), comparing the current boxed `InSet` with the sorted-array binary search (local, indicative; consistent CI numbers to be regenerated via `InExpressionBenchmark`): | set size | boxed `InSet` (ms) | sorted-array `binarySearch` (ms) | |---|---|---| | 20 | 107 | 40 | | 100 | 95 | 54 | | 500 | 151 | 82 | ~1.7–2.7x faster, with no per-row allocation. ### Does this PR introduce _any_ user-facing change? No. Query results are unchanged; this only changes the generated code for `InSet` membership. The new internal flag defaults to the optimized path. ### How was this patch tested? New and existing unit tests in `PredicateSuite`: - correctness across `Byte`/`Short`/`Int`/`Long`/`Date`/`Timestamp`/`TimestampNTZ` for value-present, value-absent, null subject, and null-in-list (three-valued logic), asserted identical with the flag on and off and with codegen on and off (interpreted `eval` vs generated code); - a codegen-inspection test asserting the generated code uses `java.util.Arrays.binarySearch`, and falls back to the boxed `Set` path when the flag is off. All `PredicateSuite` tests pass, including the pre-existing `NaN`/special-float cases, confirming the excluded `Float`/`Double` path is untouched. ### Was this patch authored or co-authored using generative AI tooling? This pull request and its description were written by Isaac. -- 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]
