apache / apache/datafusion-comet

WideDecimalBinaryExpr: write nulls directly for overflow

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

Description

### What is the problem the feature request solves?

#5044 records overflow during `WideDecimalBinaryExpr` evaluation with a `Cell`, allowing no-overflow batches to skip `null_if_overflow_precision`. Overflowing batches still write an `i128::MAX` sentinel and then run the allocating null-masking pass.

Review benchmarks show that the common no-overflow path improves by 10% to 13%, but sparse overflow is 1.6% slower, dense overflow is 5.0% slower, and overflow at the end of the batch is 3.7% slower. Passing `&Cell` into the `try_binary` closure may inhibit optimization of the kernel loop, while the overflow path still pays for the masking pass.

### Describe the potential solution

Write the null validity bit directly when overflow is detected, eliminating the sentinel, `Cell`, and follow-up masking pass.

Preserve the existing behavior:

- LEGACY and TRY overflow returns null.
- ANSI overflow returns an error.
- Scalar × Scalar evaluation returns a null `Decimal128` scalar.
- Existing input nulls remain null.

Benchmark no-overflow, sparse-overflow, dense-overflow, and last-row-overflow shapes against #5044.

### Additional context

This is a follow-up to #5044. The corresponding `DecimalRescaleCheckOverflow` cleanup is tracked separately in #5094.

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.