Skip to content

fix: handle float-to-integer overflow boundary - #813

Draft
Pybsama wants to merge 1 commit into
bytedance:mainfrom
Pybsama:codex/saturate-float-cast-boundary
Draft

fix: handle float-to-integer overflow boundary#813
Pybsama wants to merge 1 commit into
bytedance:mainfrom
Pybsama:codex/saturate-float-cast-boundary

Conversation

@Pybsama

@Pybsama Pybsama commented Aug 1, 2026

Copy link
Copy Markdown

What problem does this PR solve?

Spark-compatible floating-point-to-integer casts currently compare the input
against the target's maximum after converting that maximum to the floating-point
source type. INT_MAX rounds to 2^31 as a float, and INT64_MAX rounds to
2^63 as a double. Equality therefore misses the first positive out-of-range
value and lets it enter an undefined C++ conversion, which can produce the
target's minimum value.

Issue Number: close #809

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 🚀 Performance improvement (optimization)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • 🔨 Refactoring (no logic changes)
  • 🔧 Build/CI or Infrastructure changes
  • 📝 Documentation only

Description

  • Derive an exactly representable exclusive upper bound from the negative
    integer minimum and use >= against that boundary.
  • Keep int32_t as the first-stage type for tinyint and smallint so Spark's
    Java-style two-stage narrowing behavior remains unchanged.
  • Share the boundary helper between the general conversion utility and the
    Spark Cast expression path.
  • Cover the last in-range floating-point value, the positive overflow boundary,
    and the negative minimum for float/double conversions to all integral widths.

Using >= max() would reject a valid double(INT_MAX). In contrast, the
negated minimum is a power of two and is exactly representable in the relevant
floating-point type, so it identifies the first positive out-of-range value
without narrowing the valid range.

Local verification:

  • ConversionsTest.*: 5 passed.
  • SparkCastExprTest.*: 23 passed.
  • Spark SQL test binary excluding two data-dependent performance tests:
    698 passed, 4 skipped, 0 failed.
  • The two excluded tests both fail before their test logic because this checkout
    does not contain data/large_slow_parse.json; no Cast or conversion test was
    excluded.
  • Repository pre-commit checks, including clang-format, C++ lint, license
    headers, secret detection, and codespell, passed.
  • A standalone UBSan reproduction confirms that the previous 2^31f conversion
    enters undefined behavior.

Performance Impact

  • No Impact: The hot path still performs one comparison against a
    compile-time-derived boundary; no allocation, loop, or additional branch is
    introduced.
  • Positive Impact: I have run benchmarks.
  • Negative Impact: Explained below (e.g., trade-off for correctness).

Release Note

Release Note:

Release Note:
- Fixed Spark-compatible float and double casts at positive integer overflow boundaries.

Checklist (For Author)

  • I have added/updated unit tests (ctest).
  • I have verified the code with a local Release build.
  • I have run clang-format / linters.
  • (Optional) I have run UBSan locally on the boundary reproduction.
  • No need to test or manual test.

Breaking Changes

  • No
  • Yes (Description: ...)

Use the exact exclusive upper bound for Spark-compatible floating-point-to-integer casts so rounded representations of INT_MAX and INT64_MAX saturate instead of entering undefined conversions.

Add shared conversion and Spark expression coverage for the last in-range value, the positive overflow boundary, and the negative minimum.

Fixes bytedance#809
@CLAassistant

CLAassistant commented Aug 1, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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.

[Bug] Spark float-to-integer casts return minimum at positive overflow boundary

2 participants