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

Support for mysql alter column add auto_increment attribute #563

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

emosbaugh
Copy link
Collaborator

Altering a column and adding auto_increment when a table is not empty gives error Error 1062: ALTER TABLE causes auto_increment resequencing, resulting in duplicate entry '1' for key. This relies on prepared statements to add auto_increment with increment value transactionally. A connection with multiStatements=true is required.

@emosbaugh emosbaugh force-pushed the mysql-autoincrement-alter-column branch 2 times, most recently from 3ccb764 to 8333cf3 Compare October 11, 2021 21:01
Signed-off-by: Ethan Mosbaugh <ethan@replicated.com>
@emosbaugh emosbaugh force-pushed the mysql-autoincrement-alter-column branch from 8333cf3 to fa74878 Compare October 28, 2022 17:04
@emosbaugh emosbaugh added the enhancement New feature or request label Oct 28, 2022
@emosbaugh
Copy link
Collaborator Author

I am concerned about enabling multi-statements here

https://github.com/go-sql-driver/mysql#multistatements

Allow multiple statements in one query. While this allows batch queries, it also greatly increases the risk of SQL injections. Only the result of the first query is returned, all other results are silently discarded.

@@ -28,6 +29,12 @@ func (m *MysqlConnection) EngineVersion() string {
}

func Connect(uri string) (*MysqlConnection, error) {
var err error
uri, err = ensureMultiStatementsTrue(uri)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suggest only supporting this if multistatements are enabled rather than appending this to the connection string.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about instead of multi-statements we add transaction to the ddl?

START TRANSACTION;
set @max = (select coalesce(max(`id`), 0) + 1 from `users`);
set @alter_statement = concat('alter table `users` change column `id` `id` int (11) not null auto_increment, auto_increment=', @max);
prepare stmt from @alter_statement; execute stmt; deallocate prepare stmt;
COMMIT;

and in code add a hardcoded statement when transaction is detected to
defer ROLLBACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants