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

SQL type for string has changed after v3.* #1606

Closed
mishamyte opened this issue Jan 21, 2022 · 6 comments
Closed

SQL type for string has changed after v3.* #1606

mishamyte opened this issue Jan 21, 2022 · 6 comments

Comments

@mishamyte
Copy link

Hi team,

A lot of appreciation to your work. Thanks a lot for this great package.

The issue

Some time ago I have started work in one opensource project. First PR is bump the project and dependencies from .NET Core 3.1 to .NET 6.

The project is a package manager and it supports different DBs, using EF Core as ORM & code-first approach.

So at stable versions there is a Pomelo.EntityFrameworkCore.MySql v3.1.0
Now I'm trying to use Pomelo.EntityFrameworkCore.MySql v6.0.0

And it seems like some type mapping logic has changed.
The problem is in a strings with a large length (e.g. 4000 symbols)

When I have launched EF migrations on a 3.1 they use longtext MySQL type for this fields.

CREATE TABLE `Packages` (
  `Key` int NOT NULL AUTO_INCREMENT,
  `Id` varchar(128) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NOT NULL,
  `Authors` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `Description` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `Downloads` bigint NOT NULL,
  `HasReadme` tinyint(1) NOT NULL,
  `Language` varchar(20) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL DEFAULT NULL,
  `Listed` tinyint(1) NOT NULL,
  `MinClientVersion` varchar(44) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL DEFAULT NULL,
  `Published` datetime(6) NOT NULL,
  `RequireLicenseAcceptance` tinyint(1) NOT NULL,
  `Summary` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `Title` varchar(256) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL DEFAULT NULL,
  `IconUrl` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `LicenseUrl` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `ProjectUrl` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `RepositoryUrl` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `RepositoryType` varchar(100) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL DEFAULT NULL,
  `Tags` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `RowVersion` timestamp(6) NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6),
  `Version` varchar(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NOT NULL,
  `IsPrerelease` tinyint(1) NOT NULL DEFAULT 0,
  `SemVerLevel` int NOT NULL DEFAULT 0,
  `OriginalVersion` varchar(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL DEFAULT NULL,
  `ReleaseNotes` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci NULL,
  `HasEmbeddedIcon` tinyint(1) NOT NULL DEFAULT 0,
   ...
);

Now on 6.0 it uses varchar(...) as a type:

CREATE TABLE `Packages` (
    `Key` int NOT NULL AUTO_INCREMENT,
    `Id` varchar(128) NOT NULL,
    `Authors` varchar(4000) NULL,
    `Description` varchar(4000) NULL,
    `Downloads` bigint NOT NULL,
    `HasReadme` tinyint(1) NOT NULL,
    `Language` varchar(20) NULL,
    `Listed` tinyint(1) NOT NULL,
    `MinClientVersion` varchar(44) NULL,
    `Published` datetime(6) NOT NULL,
    `RequireLicenseAcceptance` tinyint(1) NOT NULL,
    `Summary` varchar(4000) NULL,
    `Title` varchar(256) NULL,
    `IconUrl` varchar(4000) NULL,
    `LicenseUrl` varchar(4000) NULL,
    `ProjectUrl` varchar(4000) NULL,
    `RepositoryUrl` varchar(4000) NULL,
    `RepositoryType` varchar(100) NULL,
    `Tags` varchar(4000) NULL,
    `RowVersion` datetime(6) NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6),
    `Version` varchar(64) NOT NULL,
    ...
);

Which causes the next error

Row size too large. The maximum row size for the used table type, not counting BLOBs, is 65535. This includes storage overhead, check the manual. You have to change some columns to TEXT or BLOBs

So the questions are:

  • In which release this behavior was changed?
  • How I could manage the target type to avoid this situation? (I've tried HasColumnType(longtext), but had no success)

Configuration info

Further technical details

MySQL version: MySQL Ver 8.0.27 for Linux on x86_64 (MySQL Community Server - GPL)
Operating system: Win10 Build 19044.1466 & Docker version 20.10.12, build e91ed57
Pomelo.EntityFrameworkCore.MySql version: 6.0.0


Thanks in advance!

@lauxjpn lauxjpn self-assigned this Jan 22, 2022
@lauxjpn
Copy link
Collaborator

lauxjpn commented Jan 22, 2022

@mishamyte What you are describing is not the default behavior.

Take a look at the following sample console app:

Program.cs
using System;
using System.Diagnostics;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

namespace IssueConsoleTemplate
{
    public class IceCream
    {
        public int IceCreamId { get; set; }
        public string NameDefault { get; set; }
        public string NameLongtext { get; set; }
        public string Name4000 { get; set; }
        public string Name4000Longtext { get; set; }
    }

    public class Context : DbContext
    {
        public DbSet<IceCream> IceCreams { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            var connectionString = "server=127.0.0.1;port=3306;user=root;password=;database=Issue1606";
            var serverVersion = ServerVersion.AutoDetect(connectionString);

            optionsBuilder
                .UseMySql(connectionString, serverVersion)
                .UseLoggerFactory(
                    LoggerFactory.Create(
                        b => b
                            .AddConsole()
                            .AddFilter(level => level >= LogLevel.Information)))
                .EnableSensitiveDataLogging()
                .EnableDetailedErrors();
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<IceCream>(
                entity =>
                {
                    entity.Property(i => i.NameDefault);

                    entity.Property(i => i.NameLongtext)
                        .HasColumnType("longtext");

                    entity.Property(i => i.Name4000)
                        .HasMaxLength(4000);

                    entity.Property(i => i.Name4000Longtext)
                        .HasColumnType("longtext")
                        .HasMaxLength(4000);

                    entity.HasData(
                        new IceCream
                        {
                            IceCreamId = 1,
                            NameDefault = "Vanilla",
                            NameLongtext = "Vanilla",
                            Name4000 = "Vanilla",
                            Name4000Longtext = "Vanilla",
                        },
                        new IceCream
                        {
                            IceCreamId = 2,
                            NameDefault = "Chocolate",
                            NameLongtext = "Chocolate",
                            Name4000 = "Chocolate",
                            Name4000Longtext = "Chocolate",
                        });
                });
        }
    }

    internal static class Program
    {
        private static void Main()
        {
            using var context = new Context();
            
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            var iceCreams = context.IceCreams
                .ToList();

            Trace.Assert(iceCreams.Count == 2);
        }
    }
}

It generates a CREATE TABLE statement that is expected:

Output (SQL)
warn: Microsoft.EntityFrameworkCore.Model.Validation[10400]
      Sensitive data logging is enabled. Log entries and exception messages may include sensitive application data; this mode should only be enabled during development.

info: Microsoft.EntityFrameworkCore.Infrastructure[10403]
      Entity Framework Core 6.0.0 initialized 'Context' using provider 'Pomelo.EntityFrameworkCore.MySql:6.0.0' with options: ServerVersion 8.0.25-mysql SensitiveDataLoggingEnabled DetailedErrorsEnabled

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (25ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE DATABASE `Issue1606`;

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (23ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      ALTER DATABASE CHARACTER SET utf8mb4;

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (140ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE TABLE `IceCreams` (
          `IceCreamId` int NOT NULL AUTO_INCREMENT,
          `NameDefault` longtext CHARACTER SET utf8mb4 NOT NULL,
          `NameLongtext` longtext CHARACTER SET utf8mb4 NOT NULL,
          `Name4000` varchar(4000) CHARACTER SET utf8mb4 NOT NULL,
          `Name4000Longtext` longtext CHARACTER SET utf8mb4 NOT NULL,
          CONSTRAINT `PK_IceCreams` PRIMARY KEY (`IceCreamId`)
      ) CHARACTER SET=utf8mb4;

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (24ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      INSERT INTO `IceCreams` (`IceCreamId`, `Name4000`, `Name4000Longtext`, `NameDefault`, `NameLongtext`)
      VALUES (1, 'Vanilla', 'Vanilla', 'Vanilla', 'Vanilla');

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (11ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      INSERT INTO `IceCreams` (`IceCreamId`, `Name4000`, `Name4000Longtext`, `NameDefault`, `NameLongtext`)
      VALUES (2, 'Chocolate', 'Chocolate', 'Chocolate', 'Chocolate');

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (11ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT `i`.`IceCreamId`, `i`.`Name4000`, `i`.`Name4000Longtext`, `i`.`NameDefault`, `i`.`NameLongtext`
      FROM `IceCreams` AS `i`

How does your Fluent API (or data annotations) for the Packages entity look like?

The project is a package manager and it supports different DBs [...]

Have are you generating a set of migrations for each EF Core provider individually, or are you generating each migration only once and then reuse it later for other providers?

@mishamyte
Copy link
Author

mishamyte commented Jan 22, 2022

@lauxjpn thanks for your answer!

It seems from your example like if I will do a configuration

    entity.Property(i => i.Name4000)
          .HasMaxLength(4000);

It would map by default to a

`Name4000` varchar(4000) CHARACTER SET utf8mb4 NOT NULL,

But it is not oblivious for me why NameDefault is mapped to a longtext.

How does your Fluent API (or data annotations) for the Packages entity look like?

Example of property configuration is here

package.Property(p => p.Description).HasMaxLength(4000);

Full entity configuration could be found by link in the first post (I have referenced a line where it start)

Have are you generating a set of migrations for each EF Core provider individually, or are you generating each migration only once and then reuse it later for other providers?

We are sharing the same entity configuration, but creating a migrations for each provider individually

@mishamyte
Copy link
Author

I will share an entity and it's configuration for topic consistency.

Package
public class Package
    {
        public int Key { get; set; }

        public string Id { get; set; }

        public NuGetVersion Version
        {
            get
            {
                // Favor the original version string as it contains more information.
                // Packages uploaded with older versions of BaGet may not have the original version string.
                return NuGetVersion.Parse(
                    OriginalVersionString != null
                        ? OriginalVersionString
                        : NormalizedVersionString);
            }

            set
            {
                NormalizedVersionString = value.ToNormalizedString().ToLowerInvariant();
                OriginalVersionString = value.OriginalVersion;
            }
        }

        public string[] Authors { get; set; }
        public string Description { get; set; }
        public long Downloads { get; set; }
        public bool HasReadme { get; set; }
        public bool HasEmbeddedIcon { get; set; }
        public bool IsPrerelease { get; set; }
        public string ReleaseNotes { get; set; }
        public string Language { get; set; }
        public bool Listed { get; set; }
        public string MinClientVersion { get; set; }
        public DateTime Published { get; set; }
        public bool RequireLicenseAcceptance { get; set; }
        public SemVerLevel SemVerLevel { get; set; }
        public string Summary { get; set; }
        public string Title { get; set; }

        public Uri IconUrl { get; set; }
        public Uri LicenseUrl { get; set; }
        public Uri ProjectUrl { get; set; }

        public Uri RepositoryUrl { get; set; }
        public string RepositoryType { get; set; }

        public string[] Tags { get; set; }

        /// <summary>
        /// Used for optimistic concurrency.
        /// </summary>
        public byte[] RowVersion { get; set; }

        public List<PackageDependency> Dependencies { get; set; }
        public List<PackageType> PackageTypes { get; set; }
        public List<TargetFramework> TargetFrameworks { get; set; }

        public string NormalizedVersionString { get; set; }
        public string OriginalVersionString { get; set; }


        public string IconUrlString => IconUrl?.AbsoluteUri ?? string.Empty;
        public string LicenseUrlString => LicenseUrl?.AbsoluteUri ?? string.Empty;
        public string ProjectUrlString => ProjectUrl?.AbsoluteUri ?? string.Empty;
        public string RepositoryUrlString => RepositoryUrl?.AbsoluteUri ?? string.Empty;
    }
Fluent configuration
public abstract class AbstractContext<TContext> : DbContext, IContext where TContext : DbContext
    {
        public const int DefaultMaxStringLength = 4000;

        public const int MaxPackageIdLength = 128;
        public const int MaxPackageVersionLength = 64;
        public const int MaxPackageMinClientVersionLength = 44;
        public const int MaxPackageLanguageLength = 20;
        public const int MaxPackageTitleLength = 256;
        public const int MaxPackageTypeNameLength = 512;
        public const int MaxPackageTypeVersionLength = 64;
        public const int MaxRepositoryTypeLength = 100;
        public const int MaxTargetFrameworkLength = 256;

        public const int MaxPackageDependencyVersionRangeLength = 256;
        
        // Another code
        
        protected override void OnModelCreating(ModelBuilder builder)
        {
            builder.Entity<Package>(BuildPackageEntity);
            builder.Entity<PackageDependency>(BuildPackageDependencyEntity);
            builder.Entity<PackageType>(BuildPackageTypeEntity);
            builder.Entity<TargetFramework>(BuildTargetFrameworkEntity);
        }
        
        private void BuildPackageEntity(EntityTypeBuilder<Package> package)
        {
            package.HasKey(p => p.Key);
            package.HasIndex(p => p.Id);
            package.HasIndex(p => new { p.Id, p.NormalizedVersionString })
                .IsUnique();

            package.Property(p => p.Id)
                .HasMaxLength(MaxPackageIdLength)
                .IsRequired();

            package.Property(p => p.NormalizedVersionString)
                .HasColumnName("Version")
                .HasMaxLength(MaxPackageVersionLength)
                .IsRequired();

            package.Property(p => p.OriginalVersionString)
                .HasColumnName("OriginalVersion")
                .HasMaxLength(MaxPackageVersionLength);

            package.Property(p => p.ReleaseNotes)
                .HasColumnName("ReleaseNotes");

            package.Property(p => p.Authors)
                .HasMaxLength(DefaultMaxStringLength)
                .HasConversion(StringArrayToJsonConverter.Instance)
                .Metadata.SetValueComparer(StringArrayComparer.Instance);

            package.Property(p => p.IconUrl)
                .HasConversion(UriToStringConverter.Instance)
                .HasMaxLength(DefaultMaxStringLength);

            package.Property(p => p.LicenseUrl)
                .HasConversion(UriToStringConverter.Instance)
                .HasMaxLength(DefaultMaxStringLength);

            package.Property(p => p.ProjectUrl)
                .HasConversion(UriToStringConverter.Instance)
                .HasMaxLength(DefaultMaxStringLength);

            package.Property(p => p.RepositoryUrl)
                .HasConversion(UriToStringConverter.Instance)
                .HasMaxLength(DefaultMaxStringLength);

            package.Property(p => p.Tags)
                .HasMaxLength(DefaultMaxStringLength)
                .HasConversion(StringArrayToJsonConverter.Instance)
                .Metadata.SetValueComparer(StringArrayComparer.Instance);

            package.Property(p => p.Description).HasMaxLength(DefaultMaxStringLength);
            package.Property(p => p.Language).HasMaxLength(MaxPackageLanguageLength);
            package.Property(p => p.MinClientVersion).HasMaxLength(MaxPackageMinClientVersionLength);
            package.Property(p => p.Summary).HasMaxLength(DefaultMaxStringLength);
            package.Property(p => p.Title).HasMaxLength(MaxPackageTitleLength);
            package.Property(p => p.RepositoryType).HasMaxLength(MaxRepositoryTypeLength);

            package.Ignore(p => p.Version);
            package.Ignore(p => p.IconUrlString);
            package.Ignore(p => p.LicenseUrlString);
            package.Ignore(p => p.ProjectUrlString);
            package.Ignore(p => p.RepositoryUrlString);

            // TODO: This is needed to make the dependency to package relationship required.
            // Unfortunately, this would generate a migration that drops a foreign key, which
            // isn't supported by SQLite. The migrations will be need to be recreated for this.
            // Consumers will need to recreate their database and reindex all their packages.
            // To make this transition easier, I'd like to finish this change:
            // https://github.com/loic-sharma/BaGet/pull/174
            //package.HasMany(p => p.Dependencies)
            //    .WithOne(d => d.Package)
            //    .IsRequired();

            package.HasMany(p => p.PackageTypes)
                .WithOne(d => d.Package)
                .IsRequired();

            package.HasMany(p => p.TargetFrameworks)
                .WithOne(d => d.Package)
                .IsRequired();

            package.Property(p => p.RowVersion).IsRowVersion();
        }
        
        // Another code
        }

@lauxjpn
Copy link
Collaborator

lauxjpn commented Jan 22, 2022

It seems from your example like if I will do a configuration

    entity.Property(i => i.Name4000)
          .HasMaxLength(4000);

It would map by default to a

`Name4000` varchar(4000) CHARACTER SET utf8mb4 NOT NULL,

That is correct (and expected).


But it is not oblivious for me why NameDefault is mapped to a longtext.

Because it has not been explicitly assigned a length (e.g. by using the .HasMaxLength() Fluent API call). Pomelo therefore has to assume, that the column might have to hold longer text than a varchar column could support and therefore chooses longtext automatically.


Example of property configuration is here

package.Property(p => p.Description).HasMaxLength(4000);

That will indeed result in a `Description` varchar(4000) column, which is expected. It is the same case as with the Name4000 column in my sample code.

You could either remove the .HasMaxLength(4000) call or add a .HasColumnType("longtext") call (that then effectively ignores the length; see the Name4000Longtext property in my sample code).

@lauxjpn
Copy link
Collaborator

lauxjpn commented Jan 22, 2022

You could either remove the .HasMaxLength(4000) call or add a .HasColumnType("longtext") call (that then effectively ignores the length; see the Name4000Longtext property in my sample code).

In case you cannot do this, because other supported providers cannot deal with this, you might want to do those changes only for Pomelo.

A simple way to do this is to check the provider and then change the model as needed at the end of the OnModelCreating method:

if (Database.ProviderName == "Pomelo.EntityFrameworkCore.MySql")
{
    foreach (var property in modelBuilder.Model.GetEntityTypes()
                 .SelectMany(e => e.GetProperties())
                 .Where(p => p.ClrType == typeof(string) &&
                             p.GetMaxLength() >= 4000)) // chose whatever threshold you want
    {
        property.SetMaxLength(null);
    }
}

Applying this to my sample code results in the following CREATE TABLE statement:

CREATE TABLE `IceCreams` (
    `IceCreamId` int NOT NULL AUTO_INCREMENT,
    `NameDefault` longtext CHARACTER SET utf8mb4 NOT NULL,
    `NameLongtext` longtext CHARACTER SET utf8mb4 NOT NULL,
    `Name4000` longtext CHARACTER SET utf8mb4 NOT NULL,
    `Name4000Longtext` longtext CHARACTER SET utf8mb4 NOT NULL,
    CONSTRAINT `PK_IceCreams` PRIMARY KEY (`IceCreamId`)
) CHARACTER SET=utf8mb4;

@mishamyte
Copy link
Author

@lauxjpn thanks a lot for your explanation. It became completely clear for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants