Skip to content

perf: vectorize integer-to-decimal cast#4939

Open
andygrove wants to merge 2 commits into
apache:mainfrom
andygrove:perf-optimize-int-to-decimal
Open

perf: vectorize integer-to-decimal cast#4939
andygrove wants to merge 2 commits into
apache:mainfrom
andygrove:perf-optimize-int-to-decimal

Conversation

@andygrove

@andygrove andygrove commented Jul 15, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Part of #4936.

Rationale for this change

CAST(<int> AS DECIMAL(p, s)) is common in TPC-DS (decimal casts are the second most frequent cast after AS DATE). The native path, cast_int_to_decimal128_internal, used a per-element Decimal128Builder loop with a null check and branch on every row.

What changes are included in this PR?

Replaces the element-wise loop with a single vectorized unary_opt pass: a value that overflows the multiply or does not fit the output precision maps to null, and the input null buffer is carried over. unary_opt applies the closure only to non-null slots, so it reproduces the loop's behavior in one pass with no sentinel and no second masking pass.

ANSI mode must raise on out-of-range values instead of nulling them. unary_opt only nulls non-null inputs that overflow, so a null count beyond the input's signals an overflow. That check is O(1) and gates a rare element-wise rescan that reports the first offending value with Spark's exact NumericValueOutOfRange error.

How are these changes tested?

Added unit tests for the legacy null-on-overflow (with nulls preserved), ANSI no-overflow, and ANSI overflow-error paths; the existing no-overflow test still passes. Output is bit-identical to main.

Because it is a single pass for every eval mode and shape, there is no regression on any shape:

cast_int_to_decimal: i32 -> dec(15,4):            12.97 µs -> 7.5 µs   (-42%)
cast_int_to_decimal: i32 -> dec(15,4), nulls:     19.22 µs -> 9.7 µs   (-50%)
cast_int_to_decimal: i64 -> dec(38,4):            13.04 µs -> 7.4 µs   (-43%)
cast_int_to_decimal: i32 -> dec(15,4) ansi:       13.02 µs -> 7.6 µs   (-42%)
cast_int_to_decimal: i64 -> dec(15,4) overflow:   19.44 µs -> 13.9 µs  (-28%)

@andygrove andygrove changed the title perf: vectorize integer-to-decimal cast perf: vectorize integer-to-decimal cast (2x faster) Jul 15, 2026
@andygrove andygrove marked this pull request as draft July 15, 2026 16:01
@andygrove andygrove force-pushed the perf-optimize-int-to-decimal branch from 6da4ddf to 94b9a45 Compare July 15, 2026 16:06
@andygrove andygrove changed the title perf: vectorize integer-to-decimal cast (2x faster) perf: vectorize integer-to-decimal cast Jul 15, 2026
cast_int_to_decimal128_internal used a per-element Decimal128Builder loop with a
null check and branch per row. Replace it with a single vectorized unary_opt
pass: a value that overflows the multiply or does not fit the output precision
maps to null, and the input null buffer is carried over.

ANSI mode must raise on out-of-range values rather than nulling them. unary_opt
only nulls non-null inputs that overflow, so a null count beyond the input's
signals an overflow; that O(1) check gates a rare element-wise rescan that
reports the first offending value with Spark's exact error.

This is a single pass for every eval mode and shape, so all shapes are faster
(28-50%) with no regression, including the overflow case. Add unit tests for the
legacy null-on-overflow, ANSI no-overflow, and ANSI overflow-error paths, plus a
criterion benchmark.

Part of apache#4936.
@andygrove andygrove force-pushed the perf-optimize-int-to-decimal branch from 94b9a45 to d62d695 Compare July 15, 2026 16:35
@andygrove andygrove marked this pull request as ready for review July 15, 2026 16:36

@mbutrovich mbutrovich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass, thanks @andygrove!


#[test]
fn test_cast_int_to_decimal128_overflow_legacy_nulls() {
// 1000 * 10^2 = 100000 does not fit precision 3 -> null (legacy). Valid values and the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

native/spark-expr/src/conversion_funcs/numeric.rs:1159-1160 (new test test_cast_int_to_decimal128_overflow_legacy_nulls) says the test exercises "the vectorized sentinel + masking path." The new implementation has no sentinel value and no second masking pass. unary_opt writes nulls directly. The comment is left over from an earlier design and will mislead the next reader.

Suggested change: replace with something like "exercises the vectorized null-on-overflow path with valid values and an input null preserved."

assert!(decimal_array.is_null(2));
}

#[test]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three new tests cover Legacy null-on-overflow, ANSI no-overflow, and ANSI overflow-error. None cover EvalMode::Try. Try shares the Legacy null branch today, so this is untested rather than broken, but Try is a distinct enum arm that a future edit could regress silently, and the PR text claims all eval modes are covered.

Suggested change: add a test_cast_int_to_decimal128_overflow_try_nulls mirroring the Legacy test with EvalMode::Try, asserting overflow maps to null and the input null is preserved.

// Single vectorized pass: a value that overflows the multiply or does not fit the output
// precision maps to null. `unary_opt` only applies the closure to non-null slots and carries
// the input null buffer over, replacing the per-element builder loop without a second pass.
let result: Decimal128Array = array.unary_opt::<_, Decimal128Type>(|v| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

native/spark-expr/src/conversion_funcs/numeric.rs: the fast path closure computes v.checked_mul(multiplier).filter(|scaled| is_validate_decimal_precision(*scaled, precision)), and the ANSI rescan recomputes the same thing inverted as checked_mul(multiplier).map(|scaled| !is_validate_decimal_precision(...)).unwrap_or(true). Two spellings of one predicate invite drift where one is updated and the other is not.

Suggested change: extract a small local closure or helper, for example:

let fits = |v: i128| -> Option<i128> {
    v.checked_mul(multiplier)
        .filter(|scaled| is_validate_decimal_precision(*scaled, precision))
};
let result: Decimal128Array = array.unary_opt::<_, Decimal128Type>(|v| fits(v.into()));
// rescan:
if fits(v).is_none() { return Err(SparkError::NumericValueOutOfRange { .. }); }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants