You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "david-mollitor-db (via GitHub)" <gi...@apache.org> on 2026/09/15 15:12:22 UTC

[PR] [SPARK-59539][SQL] Avoid per-row autoboxing in InSet code generation for integral and date/time types [spark]

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: 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-59539][SQL] Avoid per-row autoboxing in InSet code generation for integral and date/time types [spark]

Posted by "david-mollitor-db (via GitHub)" <gi...@apache.org>.
david-mollitor-db commented on PR #58828:
URL: https://github.com/apache/spark/pull/58828#issuecomment-5944163277

   Gentle nudge on this one, @uros-b -- no urgency at all. It stays small and self-contained (boxing-free `InSet` codegen for integral and date/time types, behind an internal flag defaulting on), and CI is green. Happy to walk through any part of it whenever you have a spare moment. Thank you!
   


-- 
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-59539][SQL] Avoid per-row autoboxing in InSet code generation for integral and date/time types [spark]

Posted by "david-mollitor-db (via GitHub)" <gi...@apache.org>.
david-mollitor-db commented on PR #58828:
URL: https://github.com/apache/spark/pull/58828#issuecomment-5750255282

   @viirya Are you able to take a look at this one too when you get a chance? THanks!


-- 
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-59539][SQL] Avoid per-row autoboxing in InSet code generation for integral and date/time types [spark]

Posted by "david-mollitor-db (via GitHub)" <gi...@apache.org>.
david-mollitor-db commented on PR #58828:
URL: https://github.com/apache/spark/pull/58828#issuecomment-5743696365

   Hi @uros-b, whenever you have a spare moment, would you be open to reviewing this one too? It's a small [SQL] change (avoiding per-row autoboxing in InSet codegen for integral and date/time types) and CI is green. No rush at all -- thank you!


-- 
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