Skip to content
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

reset auto increment counter on dolt_reset('--hard') #8319

Merged
merged 8 commits into from
Sep 4, 2024
Merged

Conversation

jycor
Copy link
Contributor

@jycor jycor commented Sep 3, 2024

When calling dolt_reset(--hard), we don't properly reload the auto increment value from the global tracker.
The bug is a side effect from maintaining auto increment value between branches. We track the last greatest auto increment value across all branches in the session, and dropping a table sets the previous auto_increment value to 0.

The fix is to get the tracker to reload values when resetting.
Some notable behavior here is that a dolt_reset over insert will not reset the auto_increment tracker.

fixes: #8272

@coffeegoddd
Copy link
Contributor

@jycor DOLT

comparing_percentages
100.000000 to 100.000000
version result total
0a912fb ok 5937457
version total_tests
0a912fb 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
4af423c ok 5937457
version total_tests
4af423c 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
c27f3a7 ok 5937457
version total_tests
c27f3a7 5937457
correctness_percentage
100.0

Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

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

Looks good!

// AcquireTableLock acquires the auto increment lock on a table, and returns a callback function to release the lock.
// Depending on the value of the `innodb_autoinc_lock_mode` system variable, the engine may need to acquire and hold
// the lock for the duration of an insert statement.
AcquireTableLock(ctx *sql.Context, tableName string) (func(), error)
// InitWithRoots fills the AutoIncrementTracker with values at the specified root.
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) Might be helpful to explain the behavior when multiple roots are present – currently it sounds like it's only for one root. For example, telling the caller that auto_inc sequence values are pulled out of each root, in order, and initialized for every table in each root.

@coffeegoddd
Copy link
Contributor

@jycor DOLT

comparing_percentages
100.000000 to 100.000000
version result total
f27708d ok 5937457
version total_tests
f27708d 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@jycor DOLT

comparing_percentages
100.000000 to 100.000000
version result total
ee59a73 ok 5937457
version total_tests
ee59a73 5937457
correctness_percentage
100.0

@jycor jycor merged commit 8dca4a5 into main Sep 4, 2024
21 checks passed
@jycor jycor deleted the james/autoinc branch September 4, 2024 08:14
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.

dolt_reset(--hard) after a drop table reserts the auto_increment counter to 1
3 participants