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 tests fail on MySQL 5.6.x community edition as DateTime milliseconds are truncated #77

Open
gentledepp opened this issue Aug 10, 2018 · 5 comments

Comments

@gentledepp
Copy link
Contributor

Hi!

I am currently struggling with getting the unit- and integration tests to work on my dev machine.
In order to help others getting started, I created a simple powershell script which installs MySQL using chocolatey. 5.x is the only MySQL version available there, this is why I came across this bug:


# install chocolatey
Set-ExecutionPolicy Bypass -Scope Process -Force; iex ((New-Object System.Net.WebClient).DownloadString('https://chocolatey.org/install.ps1'))

#install mysql
cinst mysql -y

#set initial root user password to be the one used by dotmim sync
mysqladmin -u root password azerty31$

Running "MySqlAllColumnsTests.OneRowFromServer" leads to an error:
The CDateTime value is
Expected: 2010-10-01T23:10:12.4000000
Actual: 2010-10-01T23:10:12.0000000'

The reason for that is that in MySQL 5.x, you need to specify the precision "6" in order to include milliseconds.
I.e. "DATETIME (6)"
see: https://dev.mysql.com/doc/refman/5.6/en/fractional-seconds.html

I added a hacky fix to "MySqlDbMetadata" to fix this UT:



        public override string GetPrecisionStringFromDbType(DbType dbType, int maxLength, byte precision, byte scale)
        {
            if (dbType == DbType.Guid)
                return "(36)";

            var typeName = GetStringFromDbType(dbType);
            if (IsTextType(typeName))
            {
                string lowerType = typeName.ToLowerInvariant();
                switch (lowerType)
                {
                    case "varchar":
                    case "char":
                    case "text":
                    case "nchar":
                    case "nvarchar":
                    case "enum":
                    case "set":
                        if (maxLength > 0)
                            return $"({maxLength})";
                        else
                            return string.Empty;
                }
                return string.Empty;
            }

            if (IsNumericType(typeName) && precision == 0)
            {
                precision = 10;
                scale = 0;
            }
            if (SupportScale(typeName) && scale == 0)
                return String.Format("({0})", precision);

            // UGLY FIX for truncated milliseconds issue with MySQL 5.x
            if (string.Equals("datetime", typeName, StringComparison.InvariantCultureIgnoreCase))
                return "(6)";

            if (!SupportScale(typeName))
                return string.Empty;

            return String.Format("({0},{1})", precision, scale);
        }

But that is obviously not perfect. Additionally, the precision on that column is set to "3" by default.

Question:
What would be the best way to fix this? Provision datetime columns as datetime (6) by default?

Note: I have very little experience with MySql so any advice is greatly welcome

@ashalkhakov
Copy link
Contributor

I also hit this, but on Linux, and with MySQL 8. Since this changes the way MySQL works by default, it might break other applications using the same database.

@gentledepp
Copy link
Contributor Author

so may need to actually specify a MySQL Dialect when creating the syncprovider.
Do you know the differences?
(Dude it would be so great if there was a CI system with all MySQL versions up and running)

@Mimetis
Copy link
Owner

Mimetis commented Aug 16, 2018

It's a known issue (I've already noticed that)

I'm thinking on a new way to make the type value comparison beetween two providers (sql to mysql and so on..)
And I have to admit it's complicated :)

@workgroupengineering
Copy link
Contributor

Hi,
another possible solution could be this.

@gentledepp
Copy link
Contributor Author

Could you please elaborate on that? I do not see a solution here.

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

No branches or pull requests

4 participants