apache / apache/datafusion-comet

Optimize `WideDecimalBinaryExpr`: skip the null-masking pass in non-ANSI mode when nothing overflows

Open
#4,943 1 comment 0 reactions 1 assignee Assigned to @peterxcli View on GitHub
area:expressions enhancement performance
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 4h
Merged PRs (30d)
198

Description

Part of #4936.

## Problem

`WideDecimalBinaryExpr` implements wide (128-bit intermediate) decimal `+ - *`, which runs on the hot path of most TPC-DS queries. It computes the result in a single fused `try_binary` pass (`native/spark-expr/src/math_funcs/wide_decimal_binary_expr.rs`), writing `i128::MAX` as an overflow sentinel in non-ANSI mode. It then unconditionally runs `null_if_overflow_precision` to convert sentinels into nulls:

```rust
let result = if eval_mode != EvalMode::Ansi {
result.null_if_overflow_precision(p_out)
} else {
result
};
```

`null_if_overflow_precision` is a second full pass that allocates a new array, and it runs even when no value overflowed, which is the common case.

## Proposed change

Run the masking pass only when a sentinel is actually present, mirroring what was done for `DecimalRescaleCheckOverflow` in #4938:

```rust
let result = if eval_mode != EvalMode::Ansi && result.values().contains(&i128::MAX) {
result.null_if_overflow_precision(p_out)
} else {
result
};
```

`contains` short-circuits at the first sentinel, so the overflow path is unaffected while the common no-overflow path skips the extra allocation. ANSI mode never produces a sentinel (it errors during the `try_binary` pass) and is unchanged.

Output stays bit-identical: when no sentinel is present, `null_if_overflow_precision` would have nulled nothing, so skipping it yields the same array.

## Notes

This is the non-ANSI counterpart to the `CheckOverflow` ANSI fast path in #4937. A review of the other decimal expressions found no remaining ANSI-detection asymmetry: `DecimalRescaleCheckOverflow` (#4938), `WideDecimalBinaryExpr`, and the decimal casts already do their ANSI overflow check inside a single cheap pass.

A criterion benchmark (no-overflow, no-overflow-with-nulls, sparse-overflow, dense-overflow, and an ANSI shape) should accompany the change to confirm the win and that overflow shapes do not regress.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.