Skip to content

Commit

Permalink
Don't enable IDENTITY_INSERT when it is not needed
Browse files Browse the repository at this point in the history
Add seed data to some test fixtures

Fixes #11115
Fixes #10653
  • Loading branch information
AndriySvyryd committed Mar 29, 2018
1 parent 2957845 commit 7430bbe
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 81 deletions.
69 changes: 67 additions & 2 deletions src/EFCore.Specification.Tests/BuiltInDataTypesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1144,9 +1144,74 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
{
modelBuilder.Entity<BinaryKeyDataType>();
modelBuilder.Entity<StringKeyDataType>();
modelBuilder.Entity<BuiltInDataTypes>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<BuiltInDataTypes>(eb =>
{
eb.HasData(
new BuiltInDataTypes
{
Id = 13,
PartitionId = 1,
TestInt16 = -1234,
TestInt32 = -123456789,
TestInt64 = -1234567890123456789L,
TestDouble = -1.23456789,
TestDecimal = -1234567890.01M,
TestDateTime = DateTime.Parse("01/01/2000 12:34:56"),
TestDateTimeOffset = new DateTimeOffset(DateTime.Parse("01/01/2000 12:34:56"), TimeSpan.FromHours(-8.0)),
TestTimeSpan = new TimeSpan(0, 10, 9, 8, 7),
TestSingle = -1.234F,
TestBoolean = true,
TestByte = 255,
TestUnsignedInt16 = 1234,
TestUnsignedInt32 = 1234565789U,
TestUnsignedInt64 = 1234567890123456789UL,
TestCharacter = 'a',
TestSignedByte = -128,
Enum64 = Enum64.SomeValue,
Enum32 = Enum32.SomeValue,
Enum16 = Enum16.SomeValue,
Enum8 = Enum8.SomeValue,
EnumU64 = EnumU64.SomeValue,
EnumU32 = EnumU32.SomeValue,
EnumU16 = EnumU16.SomeValue,
EnumS8 = EnumS8.SomeValue
});
eb.Property(e => e.Id).ValueGeneratedNever();
});
modelBuilder.Entity<BuiltInDataTypesShadow>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<BuiltInNullableDataTypes>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<BuiltInNullableDataTypes>(eb =>
{
eb.HasData(
new BuiltInNullableDataTypes
{
Id = 13,
PartitionId = 1,
TestNullableInt16 = -1234,
TestNullableInt32 = -123456789,
TestNullableInt64 = -1234567890123456789L,
TestNullableDouble = -1.23456789,
TestNullableDecimal = -1234567890.01M,
TestNullableDateTimeOffset = new DateTimeOffset(new DateTime(), TimeSpan.FromHours(-8.0)),
TestNullableTimeSpan = new TimeSpan(0, 10, 9, 8, 7),
TestNullableSingle = -1.234F,
TestNullableBoolean = true,
TestNullableByte = 255,
TestNullableUnsignedInt16 = 1234,
TestNullableUnsignedInt32 = 1234565789U,
TestNullableUnsignedInt64 = 1234567890123456789UL,
TestNullableCharacter = 'a',
TestNullableSignedByte = -128,
Enum64 = Enum64.SomeValue,
Enum32 = Enum32.SomeValue,
Enum16 = Enum16.SomeValue,
Enum8 = Enum8.SomeValue,
EnumU64 = EnumU64.SomeValue,
EnumU32 = EnumU32.SomeValue,
EnumU16 = EnumU16.SomeValue,
EnumS8 = EnumS8.SomeValue
});
eb.Property(e => e.Id).ValueGeneratedNever();
});
modelBuilder.Entity<BuiltInNullableDataTypesShadow>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<BinaryForeignKeyDataType>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<StringForeignKeyDataType>().Property(e => e.Id).ValueGeneratedNever();
Expand Down
150 changes: 104 additions & 46 deletions src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,68 +112,126 @@ public abstract class OwnedQueryFixtureBase : SharedStoreFixtureBase<DbContext>

protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
{
modelBuilder.Entity<OwnedPerson>().OwnsOne(p => p.PersonAddress).OwnsOne(a => a.Country);
modelBuilder.Entity<Branch>().OwnsOne(p => p.BranchAddress).OwnsOne(a => a.Country);
modelBuilder.Entity<LeafA>().OwnsOne(p => p.LeafAAddress).OwnsOne(a => a.Country);
modelBuilder.Entity<LeafB>().OwnsOne(p => p.LeafBAddress).OwnsOne(a => a.Country);
}

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
{
return base.AddOptions(builder).ConfigureWarnings(wcb => wcb.Throw());
}
modelBuilder.Entity<OwnedPerson>(eb =>
{
eb.HasData(new OwnedPerson
{
Id = 1
});

protected override void Seed(DbContext context)
{
context.Set<OwnedPerson>().AddRange(
new OwnedPerson
eb.OwnsOne(p => p.PersonAddress, ab =>
{
PersonAddress = new OwnedAddress
ab.HasData(new
{
Country = new OwnedCountry { Name = "USA" }
}
},
new Branch
{
PersonAddress = new OwnedAddress
OwnedPersonId = 1
}, new
{
OwnedPersonId = 2
}, new
{
Country = new OwnedCountry { Name = "USA" }
},
BranchAddress = new OwnedAddress
OwnedPersonId = 3
}, new
{
Country = new OwnedCountry { Name = "Canada" }
}
},
new LeafA
OwnedPersonId = 4
});

ab.OwnsOne(a => a.Country).HasData(new
{
OwnedAddressOwnedPersonId = 1,
Name = "USA"
}, new
{
OwnedAddressOwnedPersonId = 2,
Name = "USA"
}, new
{
OwnedAddressOwnedPersonId = 3,
Name = "USA"
}, new
{
OwnedAddressOwnedPersonId = 4,
Name = "USA"
});
});
});

modelBuilder.Entity<Branch>(eb =>
{
eb.HasData(new Branch
{
Id = 2
});

eb.OwnsOne(p => p.BranchAddress, ab =>
{
PersonAddress = new OwnedAddress
ab.HasData(new
{
BranchId = 2
},new
{
Country = new OwnedCountry { Name = "USA" }
},
BranchAddress = new OwnedAddress
BranchId = 3
});

ab.OwnsOne(a => a.Country).HasData(new
{
Country = new OwnedCountry { Name = "Canada" }
},
LeafAAddress = new OwnedAddress
OwnedAddressBranchId = 2,
Name = "Canada"
},new
{
Country = new OwnedCountry { Name = "Mexico" }
}
},
new LeafB
OwnedAddressBranchId = 3,
Name = "Canada"
});
});
});

modelBuilder.Entity<LeafA>(eb =>
{
eb.HasData(new LeafA
{
PersonAddress = new OwnedAddress
Id = 3
});

eb.OwnsOne(p => p.LeafAAddress, ab =>
{
ab.HasData(new
{
Country = new OwnedCountry { Name = "USA" }
},
LeafBAddress = new OwnedAddress
LeafAId = 3
});

ab.OwnsOne(a => a.Country).HasData(new
{
Country = new OwnedCountry { Name = "Panama" }
}
OwnedAddressLeafAId = 3,
Name = "Mexico"
});
});
});

context.SaveChanges();
modelBuilder.Entity<LeafB>(eb =>
{
eb.HasData(new LeafB
{
Id = 4
});

eb.OwnsOne(p => p.LeafBAddress, ab =>
{
ab.HasData(new
{
LeafBId = 4
});

ab.OwnsOne(a => a.Country).HasData(new
{
OwnedAddressLeafBId = 4,
Name = "Panama"
});
});
});
}

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).ConfigureWarnings(wcb => wcb.Throw());

public override DbContext CreateContext()
{
var context = base.CreateContext();
Expand Down
42 changes: 17 additions & 25 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using System.Text.RegularExpressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Migrations.Operations;
using Microsoft.EntityFrameworkCore.SqlServer.Internal;
Expand Down Expand Up @@ -1045,23 +1044,7 @@ protected override void Generate(
Check.NotNull(operation, nameof(operation));
Check.NotNull(builder, nameof(builder));

var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));

builder
.Append("IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(")
.Append(
stringTypeMapping.GenerateSqlLiteral(
Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)))
.AppendLine("))");

using (builder.Indent())
{
builder
.Append("SET IDENTITY_INSERT ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema))
.Append(" ON")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
}
GenerateIdentityInsert(builder, operation, on: true);

var sqlBuilder = new StringBuilder();
((SqlServerUpdateSqlGenerator)Dependencies.UpdateSqlGenerator).AppendBulkInsertOperation(
Expand All @@ -1071,23 +1054,32 @@ protected override void Generate(

builder.Append(sqlBuilder.ToString());

GenerateIdentityInsert(builder, operation, on: false);

builder.EndCommand();
}

private void GenerateIdentityInsert(MigrationCommandListBuilder builder, InsertDataOperation operation, bool on)
{
var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));

builder
.Append("IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(")
.Append(
stringTypeMapping.GenerateSqlLiteral(
Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)))
.Append("IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE")
.Append(" [name] IN (")
.Append(string.Join(", ", operation.Columns.Select(stringTypeMapping.GenerateSqlLiteral)))
.Append(") AND [object_id] = OBJECT_ID(")
.Append(stringTypeMapping.GenerateSqlLiteral(
Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)))
.AppendLine("))");

using (builder.Indent())
{
builder
.Append("SET IDENTITY_INSERT ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema))
.Append(" OFF")
.Append(on ? " ON" : " OFF")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
}

builder.EndCommand();
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,61 @@ public OwnedQueryInMemoryTest(OwnedQueryInMemoryFixture fixture, ITestOutputHelp
//TestLoggerFactory.TestOutputHelper = testOutputHelper;
}

public override void No_ignored_include_warning_when_implicit_load()
{
base.No_ignored_include_warning_when_implicit_load();
}

public class OwnedQueryInMemoryFixture : OwnedQueryFixtureBase
{
protected override ITestStoreFactory TestStoreFactory => InMemoryTestStoreFactory.Instance;

// #11474
protected override void Seed(DbContext context)
{
context.Set<OwnedPerson>().AddRange(
new OwnedPerson
{
PersonAddress = new OwnedAddress
{
Country = new OwnedCountry { Name = "USA" }
}
},
new Branch
{
PersonAddress = new OwnedAddress
{
Country = new OwnedCountry { Name = "USA" }
},
BranchAddress = new OwnedAddress
{
Country = new OwnedCountry { Name = "Canada" }
}
},
new LeafA
{
PersonAddress = new OwnedAddress
{
Country = new OwnedCountry { Name = "USA" }
},
BranchAddress = new OwnedAddress
{
Country = new OwnedCountry { Name = "Canada" }
},
LeafAAddress = new OwnedAddress
{
Country = new OwnedCountry { Name = "Mexico" }
}
},
new LeafB
{
PersonAddress = new OwnedAddress
{
Country = new OwnedCountry { Name = "USA" }
},
LeafBAddress = new OwnedAddress
{
Country = new OwnedCountry { Name = "Panama" }
}
});

context.SaveChanges();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ public override void Query_for_base_type_loads_all_owned_navs()
{
base.Query_for_base_type_loads_all_owned_navs();


// See issue #10067
AssertSql(
@"SELECT [o].[Id], [o].[Discriminator], [t].[Id], [t0].[Id], [t0].[LeafBAddress_Country_Name], [t1].[Id], [t2].[Id], [t2].[LeafAAddress_Country_Name], [t3].[Id], [t4].[Id], [t4].[BranchAddress_Country_Name], [t5].[Id], [t6].[Id], [t6].[PersonAddress_Country_Name]
Expand Down
Loading

0 comments on commit 7430bbe

Please sign in to comment.