Skip to content

GH-46557: [C++][R] Check overflow in float to int casts that allow truncation - #52469

Open
advitrocks9 wants to merge 3 commits into
apache:mainfrom
advitrocks9:fix-float-int-cast-overflow
Open

advitrocks9 wants to merge 3 commits into
apache:mainfrom
advitrocks9:fix-float-int-cast-overflow

Conversation

@advitrocks9

@advitrocks9 advitrocks9 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

A float to int cast with allow_float_truncate=true and allow_int_overflow=false never checks the range. Out of range values come back as garbage instead of an error. Casting float64 to int32 on x86-64:

1e308  -> -2147483648
3e9    -> -2147483648
-3e9   -> -2147483648
NaN    -> -2147483648

The same values cast from int64 fail with Integer value 3000000000 not in range.

What changes are included in this PR?

Once truncation is allowed, CastFloatingToInteger only checks for truncation, so nothing reads allow_int_overflow. This adds a WasOutOfRange check next to WasTruncated and runs it in that case, through the same block loop. NaN and infinities count as out of range.

The default safe cast is unchanged.

R's integer %/% casts every row, including rows it then masks, so it now also passes allow_int_overflow = TRUE to keep x %/% 0L returning NA.

Are these changes tested?

Yes. Cast.FloatingToIntOverflow covers int8, uint8 and the 64-bit edges for float16, float32 and float64, with and without truncation. It fails without the fix. arrow-compute-scalar-cast-test passes under ASAN and UBSAN. I couldn't run the R tests locally.

Are there any user-facing changes?

Yes. These casts now raise on out of range, NaN and infinite values, including R's as.integer() and as.integer64() in dplyr queries.

This PR contains a "Critical Fix". These casts silently produced incorrect values.

Copilot AI balanced review requested due to automatic review settings October 9, 2026 01:20

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #46557 has been automatically assigned in GitHub to PR creator.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #46557 has been automatically assigned in GitHub to PR creator.

Copilot AI balanced review requested due to automatic review settings October 9, 2026 01:40

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #46557 has been automatically assigned in GitHub to PR creator.

Copilot AI balanced review requested due to automatic review settings October 9, 2026 02:35

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants