From 64244ea8a447a441e20d00b812df0b5d7f29b793 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 25 Aug 2021 15:33:32 -0700 Subject: [PATCH] Don't use MERGE for single row insert with non-identity store-generated key Don't join the temporary table with the target table if only key values are store-generated Fixes #25712 Fixes #25345 --- .../Update/UpdateSqlGenerator.cs | 1 - .../Internal/SqlServerUpdateSqlGenerator.cs | 79 ++++++++++++------- .../BatchingTest.cs | 4 + .../Query/QueryBugsTest.cs | 3 +- .../UpdatesSqlServerTest.cs | 18 ++--- 5 files changed, 61 insertions(+), 44 deletions(-) diff --git a/src/EFCore.Relational/Update/UpdateSqlGenerator.cs b/src/EFCore.Relational/Update/UpdateSqlGenerator.cs index 2945347f863..3496434b37c 100644 --- 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 index 0d5315551ad..5bb527ad384 100644 --- 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 index d2d1d63d37f..0e61b60e550 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs @@ -20,6 +20,7 @@ public class BatchingTest : IClassFixture 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 index 2bd3bbc7511..911c724d7f1 100644 --- 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 index 3c75d4cdfaa..83aaf69e605 100644 --- 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()