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

Mysql exception using SystemMethods.NewGuid #1590

Closed
spiana opened this issue Apr 28, 2022 · 8 comments
Closed

Mysql exception using SystemMethods.NewGuid #1590

spiana opened this issue Apr 28, 2022 · 8 comments
Assignees
Labels
db:mysql MySQL database
Milestone

Comments

@spiana
Copy link

spiana commented Apr 28, 2022

Describe the bug
Mysql exception using SystemMethods.NewGuid

To Reproduce
create a table with a column of type varchar using default SystemMethods.NewGuid
Create
.Table("test")
.WithColumn("Id").AsInt32().PrimaryKey().Identity()
.WithColumn("code").AsString(50).NotNullable()
.WithColumn("uuid").AsString(50).WithDefault(SystemMethods.NewGuid);

When you execute the migration you will get the following Exception

MySql.Data.MySqlClient.MySqlException
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'SELECT UUID()), CONSTRAINT PK_test PRIMARY KEY (Id)) ENGINE = INNODB' at line 1
at MySql.Data.MySqlClient.MySqlStream.ReadPacket()
at MySql.Data.MySqlClient.NativeDriver.GetResult(Int32& affectedRow, Int64& insertedId)
at MySql.Data.MySqlClient.Driver.NextResult(Int32 statementId, Boolean force)
at MySql.Data.MySqlClient.MySqlDataReader.NextResult()
at MySql.Data.MySqlClient.MySqlCommand.ExecuteReader(CommandBehavior behavior)
at MySql.Data.MySqlClient.MySqlCommand.ExecuteReader()
at MySql.Data.MySqlClient.MySqlCommand.ExecuteNonQuery()
at FluentMigrator.Runner.Processors.MySql.MySqlProcessor.Process(String sql)
at FluentMigrator.Runner.Processors.ProcessorBase.Process(CreateTableExpression expression)
at FluentMigrator.Expressions.CreateTableExpression.ExecuteWith(IMigrationProcessor processor)
at FluentMigrator.Runner.MigrationRunner.<>c__DisplayClass80_0.b__1()
at FluentMigrator.Runner.StopWatch.Time(Action action)
at FluentMigrator.Runner.MigrationRunner.ExecuteExpressions(ICollection1 expressions) at FluentMigrator.Runner.MigrationRunner.ExecuteMigration(IMigration migration, Action2 getExpressions)
at FluentMigrator.Runner.MigrationRunner.ApplyMigrationUp(IMigrationInfo migrationInfo, Boolean useTransaction)
at FluentMigrator.Runner.MigrationRunner.MigrateUp(Int64 targetVersion, Boolean useAutomaticTransactionManagement)
at FluentMigrator.Runner.MigrationRunner.MigrateUp(Boolean useAutomaticTransactionManagement)
at FluentMigrator.Runner.MigrationRunner.MigrateUp()

Information (please complete the following information):

  • OS: MAC, docker with ubunto.
  • Platform .NET6
  • FluentMigrator version 3.3.1
  • Database Management System MYSQL
@jzabroski
Copy link
Collaborator

I am not a MySQL expert, but it looks like MySQL changed the rules regarding allowing functions/expressions as default values in:

That said, it looks like the MySql Generator is also spuriously adding a "SELECT" in front of the UUID() value:

https://github.com/fluentmigrator/fluentmigrator/blame/9c87d698b161b3a1bb596a5624c02891a64459c5/src/FluentMigrator.Runner.MySql/Generators/MySql/MySqlQuoter.cs#L47

@spiana - what version of MySQL are you using? It sounds like even if we change this , if you're not on MySQL 8.0.13 or later, it still won't work.

@fubar-coder - any idea why (SELECT UUID()) is generated here?

Two review questions:

  1. Does MySQL even allow a SELECT statement inside a default constraint, as in (SELECT UUID())
  2. In reading KRISTIAN KÖHNTOPP 's - SEPTEMBER 22, 2020 blog post, MySQL: ALTER TABLE for UUID, he notes that UUID() sorts poorly, and so is unlikely a good default value for any column that is to be used for indexing. He recommends instead using ``

The documentation for MySQL UUID values is longwinded and dense. See: https://dev.mysql.com/doc/refman/8.0/en/miscellaneous-functions.html#function_uuid-to-bin - the takeaways are:

  • UUID() is "... function is unsafe for statement-based replication."
  • Since UUID() generates a "UUID Version 1" value, and those values are prefixed with the Server Id portion, they are poor for indexing. and a workaround is to call (BIN_TO_UUID(UUID_TO_BIN(@uuid,1),1)) - but it is not clear to me if this will cause IS_UUID() to fail. **I think IS_UUID() will fail in cases where the Server Id part of the original UUID is not a valid timestamp id part in a MySQL UUID. However, the documentation for IS_UUID() only says it checks for valid hex characters and Guid format: **

@fubar-coder
Copy link
Member

@jzabroski Sorry, no idea

@jzabroski
Copy link
Collaborator

All good. I was able to stand up MySQL to test.

The following works well:

drop table if exists scientist;
create table scientist (id integer, firstname varchar(100), lastname varchar(100), sid varchar(50) default (BIN_TO_UUID(UUID_TO_BIN(uuid(),1),1)));
insert into scientist (id, firstname, lastname, sid) values (1, 'albert', 'einstein', default);
insert into scientist (id, firstname, lastname, sid) values (2, 'isaac', 'newton', default);
insert into scientist (id, firstname, lastname, sid) values (3, 'marie', 'curie', default);
select * from scientist;

as does

drop table if exists scientist;
create table scientist (id integer, firstname varchar(100), lastname varchar(100), sid varchar(50) default (uuid()));
insert into scientist (id, firstname, lastname, sid) values (1, 'albert', 'einstein', default);
insert into scientist (id, firstname, lastname, sid) values (2, 'isaac', 'newton', default);
insert into scientist (id, firstname, lastname, sid) values (3, 'marie', 'curie', default);
select * from scientist;

I think, based on my reading, that UUID() may be preferable by default, perhaps with an FAQ added in the docs for now for using the RawSql helper to use a more complex expression like (BIN_TO_UUID(UUID_TO_BIN(uuid(),1),1))

@jzabroski jzabroski self-assigned this May 3, 2022
@jzabroski
Copy link
Collaborator

An additional point is that Microsoft T-SQL NEWID() documentation notes:

Remarks

NEWID() is compliant with RFC4122.

In addition, RFC4122 Section 4.1.1 notes:

Interoperability, in any form, with variants other than the one
defined here is not guaranteed, and is not likely to be an issue in
practice.

Also, RC4122 Section 4.1.3 describes the Version 1 format that MySQL uses:

   Msb0  Msb1  Msb2  Msb3   Version  Description

    0     0     0     1        1     The time-based version
                                     specified in this document.

@jzabroski jzabroski added this to the 4.0.0 milestone May 3, 2022
@spiana
Copy link
Author

spiana commented May 3, 2022

@jzabroski I am now testing using Mysql version 8.0.27 but I have encountered the same problem on version 5.7.

@jzabroski
Copy link
Collaborator

@spiana I don't think 5.7 supports it, period. That said, SELECT UUID() is incorrect. What's crazy is you are the first persion to report this in 4+ years and it's been broken the whole time 😆

I'll address #1156 issues as part of 4.0.0 as well. Hope to finalize release today or tomorrow.

@jzabroski
Copy link
Collaborator

@spiana Just wondering if you know otherwise as to whether 5.7 should support this. I'm not a MySQL expert. The MySQL docs are super longwinded, like reading a John Updike novel.

@spiana
Copy link
Author

spiana commented May 3, 2022

@jzabroski. No, you're probably right, I got confused about the version. I also found some documentation where it says that functions as default were introduced in version 8.0.13.
Thanks for the fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
db:mysql MySQL database
Projects
None yet
Development

No branches or pull requests

3 participants