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.

Sign up for GitHub

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

Don't use a temporary table for batching inserts. #8806

Merged
merged 1 commit into from
Jun 9, 2017
Merged

Don't use a temporary table for batching inserts. #8806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 57 additions & 64 deletions src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ public virtual ResultSetMapping AppendBulkInsertOperation(
IReadOnlyList<ModificationCommand> modificationCommands,
int commandPosition)
{
Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
Check.NotEmpty(modificationCommands, nameof(modificationCommands));

if (modificationCommands.Count == 1
&& modificationCommands[0].ColumnModifications.All(o =>
!o.IsKey
Expand Down Expand Up @@ -140,7 +137,6 @@ private ResultSetMapping AppendBulkInsertWithoutServerValues(
}

private const string InsertedTableBaseName = "@inserted";
private const string ToInsertTableBaseName = "@toInsert";
private const string ToInsertTableAlias = "i";
private const string PositionColumnName = "_Position";
private const string PositionColumnDeclaration = "[" + PositionColumnName + "] [int]";
Expand All @@ -154,28 +150,6 @@ private ResultSetMapping AppendBulkInsertWithServerValues(
List<ColumnModification> keyOperations,
List<ColumnModification> readOperations)
{
AppendDeclareTable(
commandStringBuilder,
ToInsertTableBaseName,
commandPosition,
writeOperations,
PositionColumnDeclaration);

commandStringBuilder.Append("INSERT INTO ").Append(ToInsertTableBaseName).Append(commandPosition);
AppendValuesHeader(commandStringBuilder, writeOperations);
AppendValues(commandStringBuilder, writeOperations, "0");
for (var i = 1; i < modificationCommands.Count; i++)
{
commandStringBuilder.Append(",").AppendLine();
AppendValues(
commandStringBuilder,
modificationCommands[i].ColumnModifications.Where(o => o.IsWrite).ToList(),
i.ToString(CultureInfo.InvariantCulture));
}
commandStringBuilder
.AppendLine(SqlGenerationHelper.StatementTerminator)
.AppendLine();

AppendDeclareTable(
commandStringBuilder,
InsertedTableBaseName,
Expand All @@ -190,10 +164,10 @@ private ResultSetMapping AppendBulkInsertWithServerValues(
commandStringBuilder,
name,
schema,
ToInsertTableBaseName,
commandPosition,
ToInsertTableAlias,
writeOperations);
modificationCommands,
writeOperations,
PositionColumnName);
AppendOutputClause(
commandStringBuilder,
keyOperations,
Expand Down Expand Up @@ -239,39 +213,61 @@ private void AppendMergeCommandHeader(
[NotNull] StringBuilder commandStringBuilder,
[NotNull] string name,
[CanBeNull] string schema,
[NotNull] string toInsertTableName,
int toInsertTableIndex,
[NotNull] string toInsertTableAlias,
[NotNull] IReadOnlyList<ColumnModification> operations)
[NotNull] IReadOnlyList<ModificationCommand> modificationCommands,
[NotNull] IReadOnlyList<ColumnModification> writeOperations,
string additionalColumns = null)
{
Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
Check.NotEmpty(name, nameof(name));
Check.NotNull(operations, nameof(operations));

commandStringBuilder.Append("MERGE ");
SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema);

commandStringBuilder
.Append(" USING ")
.Append(toInsertTableName)
.Append(toInsertTableIndex.ToString(CultureInfo.InvariantCulture))
.Append(" AS ").Append(toInsertTableAlias).AppendLine(" ON 1=0")
.Append(" USING (");

AppendValuesHeader(commandStringBuilder, writeOperations);
AppendValues(commandStringBuilder, writeOperations, "0");
for (var i = 1; i < modificationCommands.Count; i++)
{
commandStringBuilder.Append(",").AppendLine();
AppendValues(
commandStringBuilder,
modificationCommands[i].ColumnModifications.Where(o => o.IsWrite).ToList(),
i.ToString(CultureInfo.InvariantCulture));
}

commandStringBuilder
.Append(") AS ").Append(toInsertTableAlias)
.Append(" (")
.AppendJoin(
writeOperations,
SqlGenerationHelper,
(sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName));
if (additionalColumns != null)
{
commandStringBuilder
.Append(", ")
.Append(additionalColumns);
}

commandStringBuilder
.Append(")")
.AppendLine(" ON 1=0")
.AppendLine("WHEN NOT MATCHED THEN");

commandStringBuilder
.Append("INSERT ")
.Append("(")
.AppendJoin(
operations,
writeOperations,
SqlGenerationHelper,
(sb, o, helper) => { helper.DelimitIdentifier(sb, o.ColumnName); })
(sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName))
.Append(")");

AppendValuesHeader(commandStringBuilder, operations);
AppendValuesHeader(commandStringBuilder, writeOperations);
commandStringBuilder
.Append("(")
.AppendJoin(
operations,
writeOperations,
toInsertTableAlias,
SqlGenerationHelper,
(sb, o, alias, helper) =>
Expand All @@ -287,9 +283,6 @@ private void AppendValues(
IReadOnlyList<ColumnModification> operations,
string additionalLiteral)
{
Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
Check.NotNull(operations, nameof(operations));

if (operations.Count > 0)
{
commandStringBuilder
Expand Down Expand Up @@ -370,16 +363,22 @@ private string GetTypeNameForCopy(IProperty property)
keyOrIndex: false,
size: null).StoreType;
}

return typeName ?? _typeMapper.FindMapping(property.ClrType).StoreType;
else
{
typeName = _typeMapper.FindMapping(property.ClrType).StoreType;
}
}
}

return property.ClrType == typeof(byte[])
&& (typeName.Equals("rowversion", StringComparison.OrdinalIgnoreCase)
|| typeName.Equals("timestamp", StringComparison.OrdinalIgnoreCase))
? (property.IsNullable ? "varbinary(8)" : "binary(8)")
: typeName;
if (property.ClrType == typeof(byte[])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in the type mapper?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very update-specific, I doubt it could be reused.

&& typeName != null
&& (typeName.Equals("rowversion", StringComparison.OrdinalIgnoreCase)
|| typeName.Equals("timestamp", StringComparison.OrdinalIgnoreCase)))
{
return property.IsNullable ? "varbinary(8)" : "binary(8)";
}

return typeName;
}

// ReSharper disable once ParameterTypeCanBeEnumerable.Local
Expand Down Expand Up @@ -419,9 +418,6 @@ private ResultSetMapping AppendInsertOperationWithServerKeys(
IReadOnlyList<ColumnModification> readOperations,
int commandPosition)
{
Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
Check.NotNull(command, nameof(command));

var name = command.TableName;
var schema = command.Schema;
var operations = command.ColumnModifications;
Expand Down Expand Up @@ -455,7 +451,7 @@ private ResultSetMapping AppendSelectCommand(
.AppendJoin(
readOperations,
SqlGenerationHelper,
(sb, o, helper) => { helper.DelimitIdentifier(sb, o.ColumnName, "t"); })
(sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName, "t"))
.Append(" FROM ");
SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, tableName, schema);
commandStringBuilder
Expand Down Expand Up @@ -495,7 +491,7 @@ private ResultSetMapping AppendSelectCommand(
/// </summary>
protected override ResultSetMapping AppendSelectAffectedCountCommand(StringBuilder commandStringBuilder, string name, string schema, int commandPosition)
{
Check.NotNull(commandStringBuilder, nameof(commandStringBuilder))
commandStringBuilder
.Append("SELECT @@ROWCOUNT")
.Append(SqlGenerationHelper.StatementTerminator).AppendLine()
.AppendLine();
Expand All @@ -508,7 +504,7 @@ protected override ResultSetMapping AppendSelectAffectedCountCommand(StringBuild
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public override void AppendBatchHeader(StringBuilder commandStringBuilder)
=> Check.NotNull(commandStringBuilder, nameof(commandStringBuilder))
=> commandStringBuilder
.Append("SET NOCOUNT ON")
.Append(SqlGenerationHelper.StatementTerminator).AppendLine();

Expand All @@ -518,9 +514,6 @@ public override void AppendBatchHeader(StringBuilder commandStringBuilder)
/// </summary>
protected override void AppendIdentityWhereCondition(StringBuilder commandStringBuilder, ColumnModification columnModification)
{
Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
Check.NotNull(columnModification, nameof(columnModification));

SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, columnModification.ColumnName);
commandStringBuilder.Append(" = ");

Expand All @@ -532,7 +525,7 @@ protected override void AppendIdentityWhereCondition(StringBuilder commandString
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
protected override void AppendRowsAffectedWhereCondition(StringBuilder commandStringBuilder, int expectedRowsAffected)
=> Check.NotNull(commandStringBuilder, nameof(commandStringBuilder))
=> commandStringBuilder
.Append("@@ROWCOUNT = ")
.Append(expectedRowsAffected.ToString(CultureInfo.InvariantCulture));
}
Expand Down
45 changes: 42 additions & 3 deletions test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,41 @@ public void Inserts_and_updates_are_batched_correctly()
}
}

[Fact]
public void Inserts_when_database_type_is_different()
{
using (var testStore = SqlServerTestStore.Create(DatabaseName + "_TypeMismatch"))
{
var options = new DbContextOptionsBuilder()
.UseSqlServer(testStore.Connection, b => b.ApplyConfiguration())
.UseInternalServiceProvider(
new ServiceCollection()
.AddEntityFrameworkSqlServer()
.BuildServiceProvider())
.Options;

using (var context = new BloggingContext(options))
{
context.Database.EnsureClean();
testStore.ExecuteNonQuery(@"
ALTER TABLE dbo.Owners
ALTER COLUMN Name nvarchar(MAX);");

var owner1 = new Owner { Id = "0", Name = "Zero" };
var owner2 = new Owner { Id = "A", Name = string.Join("", Enumerable.Repeat('A', 900)) };
context.Owners.Add(owner1);
context.Owners.Add(owner2);

context.SaveChanges();
}

using (var context = new BloggingContext(options))
{
Assert.Equal(2, context.Owners.Count());
}
}
}

private void AssertDatabaseState(bool clientOrder, List<Blog> expectedBlogs, DbContextOptions options)
{
expectedBlogs = clientOrder
Expand Down Expand Up @@ -171,7 +206,11 @@ public BloggingContext(DbContextOptions options)

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Owner>().Property(e => e.Version).IsConcurrencyToken().ValueGeneratedOnAddOrUpdate();
modelBuilder.Entity<Owner>(b =>
{
b.Property(e => e.Version).IsConcurrencyToken().ValueGeneratedOnAddOrUpdate();
b.Property(e => e.Name).HasColumnType("nvarchar(450)");
});
modelBuilder.Entity<Blog>(b =>
{
b.Property(e => e.Id).HasDefaultValueSql("NEWID()");
Expand All @@ -187,14 +226,14 @@ private class Blog
{
public Guid Id { get; set; }
public int Order { get; set; }
public int? OwnerId { get; set; }
public string OwnerId { get; set; }
public Owner Owner { get; set; }
public byte[] Version { get; set; }
}

public class Owner
{
public int Id { get; set; }
public string Id { get; set; }
public string Name { get; set; }
public byte[] Version { get; set; }
}
Expand Down
Loading