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

Sqlite AutoIncrement with other primary keys broken in 5.x #1737

Closed
volkanceylan opened this issue Feb 20, 2024 · 9 comments
Closed

Sqlite AutoIncrement with other primary keys broken in 5.x #1737

volkanceylan opened this issue Feb 20, 2024 · 9 comments
Assignees
Labels

Comments

@volkanceylan
Copy link

volkanceylan commented Feb 20, 2024

Describe the bug
A migration for a table that includes an Identity column and one or more other primary key columns are broken for Sqlite in 5.0.0 and 5.1.0 versions.

To Reproduce
The following program works fine with FluentMigrator.Runner 3.3.2 and Microsoft.Data.Sqlite 8.0.2:

using FluentMigrator;
using FluentMigrator.Runner;
using Microsoft.Extensions.DependencyInjection;

namespace SqliteAutoIncrement;

class Program
{
    const string dbName = @".\mydb.db";
    const string connectionString = $"Data Source={dbName}";

    static void Main()
    {
        File.Delete(dbName);

        var serviceProvider = new ServiceCollection()
            .AddFluentMigratorCore()
            .ConfigureRunner(rb =>
            {
                rb.AddSQLite()
                  .WithGlobalConnectionString(connectionString)
                  .ScanIn(typeof(Program).Assembly).For.Migrations();
            })
            .BuildServiceProvider(false);

        var runner = serviceProvider.GetRequiredService<IMigrationRunner>();
        runner.MigrateUp();
    }
}

[Migration(1)]
public class MyMigration: AutoReversingMigration
{
    public override void Up()
    {
        Create.Table("MyTable")
            .WithColumn("Id").AsInt32().Identity().NotNullable()
            .WithColumn("Key1").AsString(100).NotNullable().PrimaryKey()
            .WithColumn("Key2").AsString(100).NotNullable().PrimaryKey();
    }
}

But if FluentMigrator 5.0.0 or 5.1.0 is used results in the following exception:

System.Exception: 'SQLite Error 1: 'near "AUTOINCREMENT": syntax error'.
While Processing:
"
CREATE TABLE "MyTable" (
   "Id" INTEGER NOT NULL AUTOINCREMENT, 
   "Key1" TEXT NOT NULL, 
   "Key2" TEXT NOT NULL, 
   CONSTRAINT "PK_MyTable" PRIMARY KEY ("Key1", "Key2")
)
"'

Expected behavior
Existing migrations should not fail. I think the old version of FluentMigrator had a workaround for SQLite for tables that had a primary key other than the identity column as Sqlite only supports AUTOINCREMENT for the primary key. I understand that the migration is not normally valid for Sqlite, but for a project that has to support many different database types, the workaround makes a bit more sense than failing only with Sqlite. If I add PrimaryKey to the Id property it fails with the message that table has more than one primary key. So the only workaround I can think of is removing PrimaryKey from the other columns and adding a secondary unique index for them only in Sqlite, but that would mean breaking the fluent builder and repeating same column statements for Sqlite only with all such migrations.

Information (please complete the following information):

  • OS: Windows 11
  • Platform: .NET 8
  • FluentMigrator version: 5.1.0
  • FluentMigrator runner: in-process runner
  • Database Management System: SQLite
  • Database Management System Version: Microsoft.Data.Sqlite 8.0.2
@jzabroski
Copy link
Collaborator

Thanks. I should be able to look at this soon. Thank you for your patience and explanation of the problem.

@schambers
Copy link
Member

I'll take this one @jzabroski, I believe I have a fix already. I'll submit a PR shortly

@schambers schambers self-assigned this Mar 15, 2024
@jzabroski
Copy link
Collaborator

If this issue is caused by the strict tables feature, it is possible I have a fix already as part of making ITypeMap injectable.

@schambers
Copy link
Member

schambers commented Mar 15, 2024

I don't believe it's caused by that. It looks like there was some changes in the sqlite column class that allows this scenario to attempt to apply autoincrement keyword to primary key, non identity columns

I can push my branch today if you'd like to take a look @jzabroski

@jzabroski
Copy link
Collaborator

Yes

schambers added a commit that referenced this issue Mar 15, 2024
@schambers
Copy link
Member

@jzabroski, let me know if you'd like for me to open a PR: 7da19ee

@jzabroski
Copy link
Collaborator

🛳️ Yes.

@schambers
Copy link
Member

For your review @jzabroski #1744

If you approve feel free to merge or I can merge later today. I've applied the 5.2.0 milestone to this fix as well

@schambers
Copy link
Member

Issue resolved and merged to main. fyi @volkanceylan

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

No branches or pull requests

3 participants