Skip to content

dolthub/dolt#9530 - Fix auto-increment overflow handling #3103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

elianddb
Copy link
Contributor

@elianddb elianddb commented Jul 16, 2025

Fixes dolthub/dolt#9530
Added bounds checking for auto-increment values to prevent overflow beyond data type limits. The fix ensures auto-increment values don't exceed type maximums (e.g., 127 for TINYINT) and correctly throws duplicate key errors instead of allowing overflow.

@elianddb elianddb force-pushed the elianddb/9530-fix-auto-increment-overflow branch from 3cc95a1 to 9b6b168 Compare July 16, 2025 21:52
@elianddb elianddb force-pushed the elianddb/9530-fix-auto-increment-overflow branch from 7ec1899 to c840e36 Compare July 25, 2025 21:40
Copy link
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor change

memory/table.go Outdated
data.autoIncVal++
nextVal := data.autoIncVal + 1
if _, inRange, err := newCol.Type.Convert(ctx, nextVal); err == nil && inRange == sql.InRange {
data.autoIncVal++
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
data.autoIncVal++
data.autoIncVal = nextVal

t.ea.TableData().autoIncVal++ // Move onto next autoIncVal
nextVal := v.(uint64) + 1
if _, inRange, err := autoCol.Type.Convert(ctx, nextVal); err == nil && inRange == sql.InRange {
t.ea.TableData().autoIncVal++
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.ea.TableData().autoIncVal++
t.ea.TableData().autoIncVal = nextVal

t.ea.TableData().autoIncVal++ // Move onto next autoIncVal
nextVal := t.ea.TableData().autoIncVal + 1
if _, inRange, err := autoCol.Type.Convert(ctx, nextVal); err == nil && inRange == sql.InRange {
t.ea.TableData().autoIncVal++
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.ea.TableData().autoIncVal++
t.ea.TableData().autoIncVal = nextVal

@elianddb elianddb force-pushed the elianddb/9530-fix-auto-increment-overflow branch 6 times, most recently from 1b05ab9 to deb57a7 Compare July 30, 2025 21:47
@elianddb elianddb changed the title dolthub/dolt#9530 - Fix auto-increment overflow handling to match MySQL behavior dolthub/dolt#9530 - Fix auto-increment overflow handling Jul 31, 2025
elianddb and others added 8 commits July 31, 2025 11:30
Added bounds checking for auto-increment values to prevent overflow beyond data type limits.
The fix ensures auto-increment values don't exceed type maximums (e.g., 127 for TINYINT)
and correctly throws duplicate key errors instead of allowing overflow.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Removed Skip: true from tests that verify auto-increment behavior at maximum values.
Our fix now allows these tests to pass:
- Insert max values (127 for TINYINT, 32767 for SMALLINT, etc.)
- Verify SHOW CREATE TABLE shows correct AUTO_INCREMENT values
- Removed redundant auto_increment_overflow_test.go file

Tests now properly validate that AUTO_INCREMENT values don't exceed type limits
and display correctly in SHOW CREATE TABLE output.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Inlined bounds checking logic directly where used instead of helper function:
- Removed canIncrementAutoIncVal() helper function from table.go
- Inlined logic in table_editor.go Insert method (2 locations)
- Inlined logic in table.go AddColumn method (1 location)
- Used existing colType.Convert() pattern for type checking

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
These tests verify that SHOW CREATE TABLE correctly displays AUTO_INCREMENT
values at integer type maximums for unsigned types:
- TINYINT UNSIGNED: AUTO_INCREMENT=255
- SMALLINT UNSIGNED: AUTO_INCREMENT=65535
- MEDIUMINT UNSIGNED: AUTO_INCREMENT=16777215
- INT UNSIGNED: AUTO_INCREMENT=4294967295
- BIGINT UNSIGNED: AUTO_INCREMENT=18446744073709551615

Related to dolthub/dolt#9530 auto-increment overflow protection fix.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@elianddb elianddb force-pushed the elianddb/9530-fix-auto-increment-overflow branch from ee78132 to 7237ec9 Compare July 31, 2025 18:31
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.

auto_increment with max integers does not error properly
2 participants