Don't use MERGE for single row insert with non-identity store-generat…

…ed key Don't join the temporary table with the target table if only key values are store-generated Fixes #25712 Fixes #25345
dotnet · Aug 27, 2021 · ffc6bb6 · ffc6bb6
1 parent 816b1bd
commit ffc6bb6
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 44 deletions.
diff --git a/src/EFCore.Relational/Update/UpdateSqlGenerator.cs b/src/EFCore.Relational/Update/UpdateSqlGenerator.cs
@@ -253,7 +253,6 @@ protected virtual ResultSetMapping AppendSelectAffectedCommand(
 
             AppendSelectCommandHeader(commandStringBuilder, readOperations);
             AppendFromClause(commandStringBuilder, name, schema);
-            // TODO: there is no notion of operator - currently all the where conditions check equality
             AppendWhereAffectedClause(commandStringBuilder, conditionOperations);
             commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator)
                 .AppendLine();

diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs
@@ -52,14 +52,20 @@ public virtual ResultSetMapping AppendBulkInsertOperation(
             int commandPosition)
         {
             var table = StoreObjectIdentifier.Table(modificationCommands[0].TableName, modificationCommands[0].Schema);
-            if (modificationCommands.Count == 1
-                && modificationCommands[0].ColumnModifications.All(
+            if (modificationCommands.Count == 1)
+            {
+                return modificationCommands[0].ColumnModifications.All(
                     o =>
                         !o.IsKey
                         || !o.IsRead
-                        || o.Property?.GetValueGenerationStrategy(table) == SqlServerValueGenerationStrategy.IdentityColumn))
-            {
-                return AppendInsertOperation(commandStringBuilder, modificationCommands[0], commandPosition);
+                        || o.Property?.GetValueGenerationStrategy(table) == SqlServerValueGenerationStrategy.IdentityColumn)
+                    ? AppendInsertOperation(commandStringBuilder, modificationCommands[0], commandPosition)
+                    : AppendInsertOperationWithServerKeys(
+                        commandStringBuilder,
+                        modificationCommands[0],
+                        modificationCommands[0].ColumnModifications.Where(o => o.IsKey).ToList(),
+                        modificationCommands[0].ColumnModifications.Where(o => o.IsRead).ToList(),
+                        0);
             }
 
             var readOperations = modificationCommands[0].ColumnModifications.Where(o => o.IsRead).ToList();
@@ -437,30 +443,45 @@ private ResultSetMapping AppendSelectCommand(
             string? schema,
             string? orderColumn = null)
         {
-            commandStringBuilder
-                .AppendLine()
-                .Append("SELECT ")
-                .AppendJoin(
-                    readOperations,
-                    SqlGenerationHelper,
-                    (sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName, "t"))
-                .Append(" FROM ");
-            SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, tableName, schema);
-            commandStringBuilder
-                .AppendLine(" t")
-                .Append("INNER JOIN ")
-                .Append(insertedTableName).Append(insertedTableIndex)
-                .Append(" i")
-                .Append(" ON ")
-                .AppendJoin(
-                    keyOperations, (sb, c) =>
-                    {
-                        sb.Append('(');
-                        SqlGenerationHelper.DelimitIdentifier(sb, c.ColumnName, "t");
-                        sb.Append(" = ");
-                        SqlGenerationHelper.DelimitIdentifier(sb, c.ColumnName, "i");
-                        sb.Append(')');
-                    }, " AND ");
+            if (readOperations.SequenceEqual(keyOperations))
+            {
+                commandStringBuilder
+                    .AppendLine()
+                    .Append("SELECT ")
+                    .AppendJoin(
+                        readOperations,
+                        SqlGenerationHelper,
+                        (sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName, "i"))
+                    .Append(" FROM ")
+                    .Append(insertedTableName).Append(insertedTableIndex).Append(" i");
+            }
+            else
+            {
+                commandStringBuilder
+                    .AppendLine()
+                    .Append("SELECT ")
+                    .AppendJoin(
+                        readOperations,
+                        SqlGenerationHelper,
+                        (sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName, "t"))
+                    .Append(" FROM ");
+                SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, tableName, schema);
+                commandStringBuilder
+                    .AppendLine(" t")
+                    .Append("INNER JOIN ")
+                    .Append(insertedTableName).Append(insertedTableIndex)
+                    .Append(" i")
+                    .Append(" ON ")
+                    .AppendJoin(
+                        keyOperations, (sb, c) =>
+                        {
+                            sb.Append('(');
+                            SqlGenerationHelper.DelimitIdentifier(sb, c.ColumnName, "t");
+                            sb.Append(" = ");
+                            SqlGenerationHelper.DelimitIdentifier(sb, c.ColumnName, "i");
+                            sb.Append(')');
+                        }, " AND ");
+            }
 
             if (orderColumn != null)
             {

diff --git a/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs b/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs
@@ -20,6 +20,7 @@ public class BatchingTest : IClassFixture<BatchingTest.BatchingTestFixture>
         public BatchingTest(BatchingTestFixture fixture)
         {
             Fixture = fixture;
+            Fixture.TestSqlLoggerFactory.Clear();
         }
 
         protected BatchingTestFixture Fixture { get; }
@@ -211,6 +212,9 @@ private void ExecuteWithStrategyInTransaction(
         protected void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
             => facade.UseTransaction(transaction.GetDbTransaction());
 
+        private void AssertSql(params string[] expected)
+            => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
+
         private class BloggingContext : PoolableDbContext
         {
             public BloggingContext(DbContextOptions options)

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs
@@ -9275,8 +9275,7 @@ WHEN NOT MATCHED THEN
 OUTPUT INSERTED.[Id], i._Position
 INTO @inserted0;
 
-SELECT [t].[Id] FROM [BaseEntities] t
-INNER JOIN @inserted0 i ON ([t].[Id] = [i].[Id])
+SELECT [i].[Id] FROM @inserted0 i
 ORDER BY [i].[_Position];");
             }
         }

diff --git a/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs
@@ -57,18 +57,12 @@ INSERT INTO [Categories] ([Id], [Name], [PrincipalId])
 @p2=NULL (Size = 4000)
 
 SET NOCOUNT ON;
-DECLARE @inserted0 TABLE ([Id] uniqueidentifier, [_Position] [int]);
-MERGE [ProductBase] USING (
-VALUES (@p0, @p1, @p2, 0)) AS i ([Bytes], [Discriminator], [ProductWithBytes_Name], _Position) ON 1=0
-WHEN NOT MATCHED THEN
-INSERT ([Bytes], [Discriminator], [ProductWithBytes_Name])
-VALUES (i.[Bytes], i.[Discriminator], i.[ProductWithBytes_Name])
-OUTPUT INSERTED.[Id], i._Position
-INTO @inserted0;
-
-SELECT [t].[Id] FROM [ProductBase] t
-INNER JOIN @inserted0 i ON ([t].[Id] = [i].[Id])
-ORDER BY [i].[_Position];");
+DECLARE @inserted0 TABLE ([Id] uniqueidentifier);
+INSERT INTO [ProductBase] ([Bytes], [Discriminator], [ProductWithBytes_Name])
+OUTPUT INSERTED.[Id]
+INTO @inserted0
+VALUES (@p0, @p1, @p2);
+SELECT [i].[Id] FROM @inserted0 i;");
         }
 
         public override void Save_replaced_principal()