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.

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

EF migration MigrationBuilder.InsertData enables IDENTITY_INSERT when it is not needed. #11115

Closed
dmytroett opened this issue Mar 1, 2018 · 2 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@dmytroett
Copy link

I'm trying to insert some data to DB via migration. I've discovered that it is possible via MigrationBuilder.InsertData() method.

My environment:
EF Core version: 2.0.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows Server 2016 Datacenter
IDE: Visual Studio 2017 Version 15.5.7

I've tried following

  1. Opened Package manager console
  2. Executed Add-Migration InsertSomeData
    Here how it looks:
    public partial class InsertSomeData : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.InsertData("Lookups", new[] { "LookupName", "Value", "Text" }, new object[] { "TestData", true, "Test description" });
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DeleteData("Lookups", "LookupName", "TestData");
        }
    }
}

The Lookups table schema:

CREATE TABLE [dbo].[Lookups](
	[Id] [int] IDENTITY(1,1) NOT NULL,
	[LookupName] [nvarchar](20) NULL,
	[Value] [nvarchar](100) NULL,
	[Text] [nvarchar](max) NULL,
	[MultiFlag] [bit] NULL,
 CONSTRAINT [PK_Lookups] PRIMARY KEY CLUSTERED 
(
	[Id] ASC
)

And it failed with following error:

Failed executing DbCommand (21ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(N'Lookups'))
    SET IDENTITY_INSERT [Lookups] ON;
INSERT INTO [Lookups] ([LookupName], [Value], [Text])
VALUES (N'TestData', 1, N'Test description');
IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(N'Lookups'))
    SET IDENTITY_INSERT [Lookups] OFF;
Error Number:545,State:1,Class:16
Explicit value must be specified for identity column in table 'Lookups' either when IDENTITY_INSERT is set to ON or when a replication user is inserting into a NOT FOR REPLICATION identity column.
System.Data.SqlClient.SqlException (0x80131904): Explicit value must be specified for identity column in table 'Lookups' either when IDENTITY_INSERT is set to ON or when a replication user is inserting into a NOT FOR REPLICATION identity column.

Stack trace:

   at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at System.Data.SqlClient.SqlCommand.RunExecuteNonQueryTds(String methodName, Boolean async, Int32 timeout, Boolean asyncWrite)
   at System.Data.SqlClient.SqlCommand.InternalExecuteNonQuery(TaskCompletionSource`1 completion, Boolean sendToPipe, Int32 timeout, Boolean asyncWrite, String methodName)
   at System.Data.SqlClient.SqlCommand.ExecuteNonQuery()
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteNonQuery(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Migrations.MigrationCommand.ExecuteNonQuery(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteNonQuery(IEnumerable`1 migrationCommands, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.UpdateDatabase(String targetMigration, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.UpdateDatabase.<>c__DisplayClass0_1.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)

The issue here is that EF enables IDENTITY_INSERT but I'm actually OK with SQL Server generating ID value for me. After investigation it seems that

IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(N'Lookups'))
    SET IDENTITY_INSERT [Lookups] ON; 

was added to resulting SQL at https://github.com/aspnet/EntityFrameworkCore/blob/927082e095e9e937b3e684ecc8f2cc5d183a8634/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs#L1057-L1091

Is there a way to disable such behavior somehow?
PS: I can create a repo for reproduction if details are not comprehensive enough.

@edwiles
Copy link

edwiles commented Mar 1, 2018

+1 for this, we're finding the current behaviour useful on our project but I can imagine situations where it would be better to leave identity insert enabled.

@ajcvickers
Copy link
Member

Notes from triage:

  • Store generated key values cannot be used when using seed data in OnModelCreating (which is the normal way of specifying seed data in EF Core 2.1). This is because EF needs to be able to find each entity in order to be able to later update it, which requires that the entities have stable, well-known keys.
  • When writing inserts directly into the migration, as is shown above, then if the types are mapped and we know that a value has been specified for an identity column we will continue to switch identity insert off. However, if no value has been specified for an identity column, then we will not switch identity insert off.
  • If we have no data about identity columns (e.g. types not mapped) then we will not switch identity insert off by default. The migration would need to add explicit calls to turn it off if it is needed in this case.

@ajcvickers ajcvickers added this to the 2.1.0 milestone Mar 2, 2018
AndriySvyryd added a commit that referenced this issue Mar 29, 2018
Add seed data to some test fixtures

Fixes #11115
Fixes #10653
AndriySvyryd added a commit that referenced this issue Mar 29, 2018
Add seed data to some test fixtures

Fixes #11115
Fixes #10653
@AndriySvyryd AndriySvyryd added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed priority-stretch labels Mar 29, 2018
@AndriySvyryd AndriySvyryd removed their assignment Mar 29, 2018
@ajcvickers ajcvickers modified the milestones: 2.1.0-preview2, 2.1.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

4 participants