Skip to content

expression: fix signed bigint subtraction overflow | tidb-test=pr/2742#68441

Open
hawkingrei wants to merge 2 commits into
pingcap:masterfrom
hawkingrei:issue-67211-bigint-sub-overflow
Open

expression: fix signed bigint subtraction overflow | tidb-test=pr/2742#68441
hawkingrei wants to merge 2 commits into
pingcap:masterfrom
hawkingrei:issue-67211-bigint-sub-overflow

Conversation

@hawkingrei
Copy link
Copy Markdown
Member

@hawkingrei hawkingrei commented May 17, 2026

What problem does this PR solve?

Issue Number: close #67211

Problem Summary:

Signed BIGINT subtraction missed the 0 - math.MinInt64 overflow boundary. TiDB could return -9223372036854775808 without warnings for (0) - a when a was -9223372036854775808, while MySQL reports an out-of-range error.

What changed and how does it work?

The signed/signed integer subtraction overflow check now treats zero the same as other non-negative left operands when subtracting a negative right operand. This preserves in-range cases such as -1 - math.MinInt64 while raising overflow for 0 - math.MinInt64.

Regression coverage was added for both row-based and vectorized BIGINT subtraction evaluation.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Unit test:

./tools/check/failpoint-go-test.sh pkg/expression -run 'TestArithmeticMinus|TestVectorizedDecimalErrOverflow' -count=1
make lint

Manual test:

  • Replayed the issue SQL locally with a temporary testkit harness before and after the fix.
  • Before the fix, (0) - a returned -9223372036854775808 without warnings.
  • After the fix, (0) - a and (1) - a return error 1690, while (-1) - a still returns 9223372036854775807.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Fix an issue where signed BIGINT subtraction could silently overflow for `0 - -9223372036854775808` instead of returning an out-of-range error.

Summary by CodeRabbit

  • Bug Fixes

    • Adjusted overflow detection logic for integer subtraction when the first operand equals zero.
  • Tests

    • Added comprehensive test cases covering integer subtraction boundary conditions with minimum integer values.
    • Added vectorized operation tests for integer subtraction overflow scenarios.

Review Change Stack

@hawkingrei hawkingrei added the AI-Correction Bugfix by AI label May 17, 2026
@ti-chi-bot ti-chi-bot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/needs-triage-completed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 17, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented May 17, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign xuhuaiyu for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

📝 Walkthrough

Walkthrough

This PR fixes a silent overflow bug where (0) - BIGINT_MIN fails to raise an out-of-range error. The fix adjusts the overflow-detection condition in builtinArithmeticMinusIntSig to include zero, and adds scalar and vectorized test coverage validating the corrected behavior across both evaluation paths.

Changes

BIGINT minus overflow edge case

Layer / File(s) Summary
Overflow detection fix and test coverage
pkg/expression/builtin_arithmetic.go, pkg/expression/builtin_arithmetic_test.go, pkg/expression/builtin_arithmetic_vec_test.go
The overflowCheck condition changes from a > 0 && b < 0 to a >= 0 && b < 0, treating zero as a boundary case. Scalar tests verify that (0) - BIGINT_MIN and other edge cases correctly raise errors or return expected results. Vectorized tests confirm the same behavior in columnar evaluation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

approved, lgtm

Suggested reviewers

  • windtalker
  • qw4990

Poem

🐰 A zero minus the coldest of ints,
Now properly warns of its borderline hints,
No silent overflow in the night,
With tests to confirm every edge is right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The PR description follows the required template, includes the issue link, problem summary, explanation of changes, test checklist with selections, and release notes.
Linked Issues check ✅ Passed The code changes directly address issue #67211 by fixing the overflow detection for 0 - math.MinInt64 in signed BIGINT subtraction, with corresponding test coverage.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the signed BIGINT subtraction overflow bug: adjusting overflow logic, adding test cases for MININT64 scenarios, and documenting boundary behavior.
Title check ✅ Passed The title clearly identifies the specific change: a fix for signed bigint subtraction overflow, matching the core objective of handling the 0 - MinInt64 boundary case.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.4914%. Comparing base (6a6eefe) to head (1bdc13d).

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #68441        +/-   ##
================================================
- Coverage   77.2762%   76.4914%   -0.7849%     
================================================
  Files          2010       1992        -18     
  Lines        555477     557613      +2136     
================================================
- Hits         429252     426526      -2726     
- Misses       125305     131043      +5738     
+ Partials        920         44       -876     
Flag Coverage Δ
integration 41.5383% <100.0000%> (+1.7442%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 60.4888% <ø> (ø)
parser ∅ <ø> (∅)
br 49.9725% <ø> (-13.0354%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hawkingrei hawkingrei changed the title expression: fix signed bigint subtraction overflow expression: fix signed bigint subtraction overflow | tidb-test=pr/2742 May 17, 2026
@hawkingrei
Copy link
Copy Markdown
Member Author

/retest-required

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

Labels

AI-Correction Bugfix by AI do-not-merge/needs-triage-completed release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Silent overflow when computing (0) - BIGINT_MIN, while others correctly throw out-of-range errors

1 participant