david-mollitor-db opened a new pull request, #58833: URL: https://github.com/apache/spark/pull/58833
> [!NOTE] > **Draft / low-priority follow-up — please do not review or merge yet.** This is stacked on **#58828 (SPARK-59539)**, which is a prerequisite. Until #58828 merges, the diff below also contains its commits; once it lands this branch will be rebased so the diff is the Decimal delta only, and a JIRA will be filed then. ### What changes were proposed in this pull request? Follow-up to #58828 (SPARK-59539), which adds a boxing-free sorted-array `binarySearch` path to `InSet` code generation for integral and date/time types. This PR extends that same path to `DecimalType`: a sorted `Decimal[]` is built once on the driver and probed with `java.util.Arrays.binarySearch` via the `Object[]` overload, which compares by `Decimal.compareTo` (== `Decimal.compare` — value-correct across scales and consistent with `Decimal.equals`, defined as `compare(d) == 0`). It reuses the existing `spark.sql.optimizer.inSetBinarySearch.enabled` flag. `Float`/`Double` (NaN, `-0.0`/`+0.0`) and `String` remain on the boxed `Set` path. ### Why are the changes needed? The generic `Set` path calls `Decimal.hashCode()` per row, which is `toBigDecimal.hashCode()` — for compact decimals (precision ≤ 18, long-backed) this **allocates a `java.math.BigDecimal` every row** just to hash. `Decimal.compare` is instead a cheap `longVal` comparison for compact same-scale values, so `binarySearch` avoids the per-row allocation. Local microbenchmark (`InSetDecimalBenchmark`, 10M rows, non-matching values, worst case): | Decimal type | N | boxed `Set` (ms) | `binarySearch` (ms) | speedup | |---|---|---|---|---| | compact `decimal(12,1)` | 20 | 478 | 122 | 3.9× | | compact `decimal(12,1)` | 100 | 415 | 136 | 3.1× | | compact `decimal(12,1)` | 200 | 412 | 146 | 2.8× | | big `decimal(30,7)` | 20 | 110 | 44 | 2.5× | | big `decimal(30,7)` | 100 | 88 | 54 | 1.6× | | big `decimal(30,7)` | 200 | 94 | 62 | 1.5× | (Local/indicative; committable numbers to be regenerated on the CI runners.) ### Does this PR introduce _any_ user-facing change? No. Query results are unchanged; only the generated `InSet` code for decimals changes. ### How was this patch tested? A new `PredicateSuite` test covering compact `decimal(12,1)` and BigDecimal-backed `decimal(30,7)` — value present/absent, null subject, and null-in-list (three-valued logic) — asserted identical with the flag on and off. The codegen-inspection test also asserts decimals emit `Arrays.binarySearch`. All `PredicateSuite` tests pass. ### 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]
