Fix: Identity insert handling for merge (InsertOrUpdate) statements in SQLite (#138)

* fix: do not use identity column when identity insert is off #131

* chore: fix flags in integration test cases

* chore: remove fire triggers flag

* chore: add integration tests for SQLite insert or update case

---------

Co-authored-by: Michal Wilski <[email protected]>

Author: triforcely

EFCore.BulkExtensions.Tests/BulkInsertOrUpdate/BulkInsertOrUpdateTests.cs

@@ -1,9 +1,10 @@
 using EFCore.BulkExtensions.SqlAdapters;
-using Microsoft.Data.SqlClient;
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using Xunit;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.EntityFrameworkCore;
 using Xunit.Abstractions;
 
 
@@ -307,8 +308,45 @@ public void BulkInsertOrUpdate_ReloadList_IsWorking(DbServerType dbServerType)
         Assert.True(ensureList[1].Id != 0);
     }
 
+    /// <summary>
+    /// Covers: https://github.com/videokojot/EFCore.BulkExtensions.MIT/issues/131
+    /// </summary>
+    [Theory]
+    [InlineData(DbServerType.SQLite)]
+    public async Task BulkInsertOrUpdate_InsertsOrUpdatesByPropertyIn(DbServerType dbType)
+    {
+        await using var db = _dbFixture.GetDb(dbType);
+
+        var bulkConfig = new BulkConfig
+        {
+            UpdateByProperties = new List<string> { nameof(UniqueItem.FirstName), nameof(UniqueItem.LastName) }
+        };
+
+        var mockFirstName = Guid.NewGuid().ToString();
+        var mockLastName = Guid.NewGuid().ToString();
+
+        const int testSetSize = 2;
+        var testSet = Enumerable.Range(0, testSetSize)
+            .Select(x => new UniqueItem()
+            {
+                FirstName = mockFirstName + x,
+                LastName = mockLastName + x
+            });
+        
+        // duplicate enumeration is intentional - I want to create set of duplicates
+        // to verify that the "insert or update" (aka merge) works correctly and
+        // allows for database generated identity columns while doing so
+        var duplicateSet = testSet.Concat(testSet).ToList();
+        
+        await db.BulkInsertOrUpdateAsync(duplicateSet, bulkConfig, null, null, CancellationToken.None);
+        
+        Assert.Equal(testSetSize, await db.UniqueItems.CountAsync());
+        // before fix, it attempted to insert identity column, so it would insert "0" in this case, as it is default(int)
+        Assert.True(await db.UniqueItems.AllAsync(x => x.Id > 0)); 
+    }
+
     public class DatabaseFixture : BulkDbTestsFixture<SimpleBulkTestsContext>
     {
         protected override string DbName => nameof(BulkInsertOrUpdateTests);
     }
-}
+}

EFCore.BulkExtensions.Tests/BulkInsertOrUpdate/SimpleBulkTestsContext.cs

@@ -11,8 +11,9 @@ public class SimpleBulkTestsContext : DbContext
 {
     public DbSet<SimpleItem> SimpleItems { get; set; } = null!;
 
-
     public DbSet<Entity_CustomColumnNames> EntityCustomColumnNames { get; set; } = null!;
+    
+    public DbSet<UniqueItem> UniqueItems { get; set; } = null!;
 
     public SimpleBulkTestsContext(DbContextOptions options)
         : base(options)
@@ -21,6 +22,9 @@ public SimpleBulkTestsContext(DbContextOptions options)
 
     protected override void OnModelCreating(ModelBuilder modelBuilder)
     {
+        modelBuilder.Entity<UniqueItem>()
+            .HasIndex(x => new { x.FirstName, x.LastName })
+            .IsUnique();
     }
 
     public List<SimpleItem> GetItemsOfBulk(Guid bulkId)
@@ -43,6 +47,17 @@ public class SimpleItem
     public string? StringProperty { get; set; }
 }
 
+public class UniqueItem
+{
+    [Key]
+    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
+    public int Id { get; set; }
+
+    public string FirstName { get; set; } = "";
+
+    public string LastName { get; set; } = "";
+}
+
 public class Entity_CustomColumnNames
 {
     [Column("Id")]

EFCore.BulkExtensions.Tests/EFCoreBulkTest.cs

@@ -10,6 +10,7 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Text.Json;
+using Microsoft.Data.SqlClient;
 using Xunit;
 
 namespace EFCore.BulkExtensions.Tests;
@@ -533,7 +534,7 @@ private static void RunInsertOrUpdate(bool isBulk, DbServerType dbServer)
         }
         if (isBulk)
         {
-            var bulkConfig = new BulkConfig() { SetOutputIdentity = true, CalculateStats = true };
+            var bulkConfig = new BulkConfig { SetOutputIdentity = true, CalculateStats = true, SqlBulkCopyOptions = SqlBulkCopyOptions.KeepIdentity};
             context.BulkInsertOrUpdate(entities, bulkConfig, (a) => WriteProgress(a));
             if (dbServer == DbServerType.SQLServer)
             {

EFCore.BulkExtensions.Tests/EFCoreBulkTestAsync.cs

@@ -8,6 +8,7 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Threading.Tasks;
+using Microsoft.Data.SqlClient;
 using Xunit;
 
 namespace EFCore.BulkExtensions.Tests;
@@ -221,7 +222,7 @@ private static async Task RunInsertOrUpdateAsync(bool isBulk, DbServerType dbSer
         }
         if (isBulk)
         {
-            var bulkConfig = new BulkConfig() { SetOutputIdentity = true, CalculateStats = true };
+            var bulkConfig = new BulkConfig() { SetOutputIdentity = true, CalculateStats = true, SqlBulkCopyOptions = SqlBulkCopyOptions.KeepIdentity};
             await context.BulkInsertOrUpdateAsync(entities, bulkConfig);
             if (dbServer == DbServerType.SQLServer)
             {

EFCore.BulkExtensions.Tests/SqlQueryBuilderSqliteTests.cs

@@ -2,16 +2,16 @@
 using System.Collections.Generic;
 using System.Linq;
 using EFCore.BulkExtensions.SqlAdapters.SQLite;
-using Xunit;
+using Microsoft.Data.SqlClient;
 
 namespace EFCore.BulkExtensions.Tests;
 
 public class SqlQueryBuilderSqliteTests
 {
     [Fact]
-    public void MergeTableInsertOrUpdateWithoutOnConflictUpdateWhereSqlTest()
+    public void MergeTableInsertOrUpdateWithoutOnConflictWithIdentityUpdateWhereSqlTest()
     {
-        TableInfo tableInfo = GetTestTableInfo();
+        TableInfo tableInfo = GetTestTableInfo(bulkCopyOptions: SqlBulkCopyOptions.KeepIdentity);
         tableInfo.IdentityColumnName = "ItemId";
         string actual = SqlQueryBuilderSqlite.InsertIntoTable(tableInfo, OperationType.InsertOrUpdate);
 
@@ -23,10 +23,25 @@ public void MergeTableInsertOrUpdateWithoutOnConflictUpdateWhereSqlTest()
         Assert.Equal(expected, actual);
     }
 
+    [Fact]
+    public void MergeTableInsertOrUpdateWithoutOnConflictWithoutIdentityUpdateWhereSqlTest()
+    {
+        TableInfo tableInfo = GetTestTableInfo();
+        tableInfo.IdentityColumnName = "ItemId";
+        string actual = SqlQueryBuilderSqlite.InsertIntoTable(tableInfo, OperationType.InsertOrUpdate);
+
+        string expected = @"INSERT INTO [Item] ([Name]) " +
+                          @"VALUES (@Name) " +
+                          @"ON CONFLICT([ItemId]) DO UPDATE SET [Name] = @Name " +
+                          @"WHERE [ItemId] = @ItemId;";
+
+        Assert.Equal(expected, actual);
+    }
+    
     [Fact]
     public void MergeTableInsertOrUpdateWithOnConflictUpdateWhereSqlTest()
     {
-        TableInfo tableInfo = GetTestTableInfo((existing, inserted) => $"{inserted}.ItemTimestamp > {existing}.ItemTimestamp");
+        TableInfo tableInfo = GetTestTableInfo((existing, inserted) => $"{inserted}.ItemTimestamp > {existing}.ItemTimestamp", SqlBulkCopyOptions.KeepIdentity);
         tableInfo.IdentityColumnName = "ItemId";
         string actual = SqlQueryBuilderSqlite.InsertIntoTable(tableInfo, OperationType.InsertOrUpdate);
 
@@ -38,7 +53,9 @@ public void MergeTableInsertOrUpdateWithOnConflictUpdateWhereSqlTest()
         Assert.Equal(expected, actual);
     }
 
-    private TableInfo GetTestTableInfo(Func<string, string, string>? onConflictUpdateWhereSql = null)
+    private TableInfo GetTestTableInfo(
+        Func<string, string, string>? onConflictUpdateWhereSql = null
+        , SqlBulkCopyOptions? bulkCopyOptions = null)
     {
         var tableInfo = new TableInfo()
         {
@@ -52,7 +69,8 @@ private TableInfo GetTestTableInfo(Func<string, string, string>? onConflictUpdat
             PrimaryKeysPropertyColumnNameDict = new Dictionary<string, string> { { nameof(Item.ItemId), nameof(Item.ItemId) } },
             BulkConfig = new BulkConfig()
             {
-                OnConflictUpdateWhereSql = onConflictUpdateWhereSql
+                OnConflictUpdateWhereSql = onConflictUpdateWhereSql,
+                SqlBulkCopyOptions = bulkCopyOptions ?? SqlBulkCopyOptions.Default
             }
         };
         const string nameText = nameof(Item.Name);

EFCore.BulkExtensions/SqlAdapters/SQLite/SqlQueryBuilderSqlite.cs

@@ -43,7 +43,7 @@ public static string InsertIntoTable(TableInfo tableInfo, OperationType operatio
         List<string> propertiesList = tableInfo.PropertyColumnNamesDict.Keys.ToList();
 
         bool keepIdentity = tableInfo.BulkConfig.SqlBulkCopyOptions.HasFlag(SqlBulkCopyOptions.KeepIdentity);
-        if (operationType == OperationType.Insert && !keepIdentity && tableInfo.HasIdentity)
+        if (!tableInfo.InsertToTempTable && !keepIdentity && tableInfo.HasIdentity)
         {
             var identityPropertyName = tableInfo.PropertyColumnNamesDict.SingleOrDefault(a => a.Value == tableInfo.IdentityColumnName).Key;
             columnsList = columnsList.Where(a => a != tableInfo.IdentityColumnName).ToList();