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

Inserted entities has different ID (DB vs instance) after SaveChanges #4080

Closed
LonDat opened this issue Dec 14, 2015 · 8 comments
Closed

Inserted entities has different ID (DB vs instance) after SaveChanges #4080

LonDat opened this issue Dec 14, 2015 · 8 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@LonDat
Copy link

LonDat commented Dec 14, 2015

Hi guys...
I have problem with inserting more entities (same class).
My environment: MSSQL 2014 (this trouble may appears on MSSQL 2012+).

Situation:
Create 500 instances of one type entity a call SaveChanges.
You generate one insert with many values and identity insert into temporary table...but:
"There is no guarantee that the order in which the changes are applied to the table and the order in which the rows are inserted into the output table or table variable will correspond." (source: https://msdn.microsoft.com/en-us/library/ms177564.aspx or http://stackoverflow.com/questions/11222043/table-valued-function-order-by-is-ignored-in-output).

I have table with identity (no big deal...default identity(1, 1)) and DbReader output is:

--ID int (PK, identity(1, 1)):
1. 508
2. 466
3. 462
4. 493
5. 504
6. 520
7. 496
8. 497
9. 494
10. 277
11. 281
12. 263
13. 476
...

This is fatal when you need continue work with instances :(

Thx for fix,
Milan

@divega
Copy link
Contributor

divega commented Dec 14, 2015

@LonDat thanks for the bug report. I would like to confirm my interpretation of the bug report is correct.

Do you mean that after we read the generated values in SaveChanges() you are seeing that the entities in the context have the wrong IDs? E.g. while the database contains:

Id   Name  
1    Juan
2    Pedro

the objects in the context are:

Id   Name
2    Juan
1    Pedro

Or is the issue that you expected the IDs of the entities to be in the same order in which you added them to the context and that isn't happening?

BTW, the two main reasons I ask are:

  1. Assuming the most voted answer to the SO question is correct, SQL Server appears to guarantee the order of insertions will be respected if the target table has an IDENTITY column (as in your case) so I would expect rows to get their identities assigned in the same order in which EF put them in the SQL statement.
  2. Which brings me to the fact that currently EF7 does not guarantee that the order of the database insert/update/delete operations (or the order of the values passed to a multi-insert) will match the order in which those changes were recorded by the DbContext.

@divega
Copy link
Contributor

divega commented Dec 14, 2015

If we are actually getting the wrong identities assigned to entities this is a possible cause:

From Ordering guarantees in SQL Server...:

INSERT queries that use SELECT with ORDER BY to populate rows guarantees how identity values are computed but not the order in which the rows are inserted

This might mean that the order or the rows in the INSERTED table is not what we expect.

@LonDat
Copy link
Author

LonDat commented Dec 14, 2015

Hi, thx for quick reply.
Your interpretation of my bug is correct (db vs context).

I tried this:
File: \src\EntityFramework.Relational\Update\AffectedCountModificationCommandBatch.cs
Line: 166 (method ConsumeResultSetWithPropagation)
Insert this:

                var x = new object[reader.FieldCount];
                var xx = reader.GetValues(x);

                System.IO.File.AppendAllLines(@"C:\temp\idx.log", new[] {$"{commandIndex:0000}: {x[0]}"});

And I get this:

1. 508
2. 466
3. 462
4. 493
5. 504
6. 520
7. 496
8. 497
9. 494
10. 277
11. 281
12. 263
13. 476

This is SQL DDL:

CREATE TABLE [dbo].[EmailMessageAddress](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [AddressId] [int] NULL,
    [AddressType] [int] NOT NULL,
    [DisplayName] [nvarchar](150) NULL,
    [MessageId] [int] NOT NULL,
 CONSTRAINT [PK_EmailMessageAddress] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

GO

ALTER TABLE [dbo].[EmailMessageAddress]  WITH CHECK ADD  CONSTRAINT [FK_EmailMessageAddress_EmailAddress_AddressId] FOREIGN KEY([AddressId])
REFERENCES [dbo].[EmailAddress] ([Id])
GO

ALTER TABLE [dbo].[EmailMessageAddress] CHECK CONSTRAINT [FK_EmailMessageAddress_EmailAddress_AddressId]
GO

ALTER TABLE [dbo].[EmailMessageAddress]  WITH CHECK ADD  CONSTRAINT [FK_EmailMessageAddress_EmailMessage_MessageId] FOREIGN KEY([MessageId])
REFERENCES [dbo].[EmailMessage] ([Id])
ON DELETE CASCADE
GO

ALTER TABLE [dbo].[EmailMessageAddress] CHECK CONSTRAINT [FK_EmailMessageAddress_EmailMessage_MessageId]
GO

It's true, that I try run generated command (from SQL profiler) run directly in SQL management (with directly typed values), I never get ID's in wrong order :(

Thx,
Milan

@divega
Copy link
Contributor

divega commented Dec 14, 2015

@LonDat Makes sense. Thanks a lot for the report!

@rowanmiller & others in triage, @AndriySvyryd and I discussed this and we believe it is a very high priority to fix.

@divega divega added this to the 7.0.0-rc2 milestone Dec 14, 2015
@ajcvickers
Copy link
Contributor

Agreed.

@LonDat
Copy link
Author

LonDat commented Dec 14, 2015

Some data from my environment...perhaps this helps.

batch.sql.txt
batch-sqlplan.sqlplan.txt
idx-order.txt

Everything is from one test...idx-order.txt is from my test in ConsumeResultSetWithPropagation described in my last post.
I hope, that attached sql plan from SQL Activity Monitor is really from this test too :)

Thx again...
Milan

@brandondahler
Copy link

To reproduce, you'll also need to add the following DDL:

CREATE TABLE [dbo].[EmailMessage](
    [Id] [int] IDENTITY(1,1) NOT NULL,
 CONSTRAINT [PK_EmailMessage] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]


CREATE TABLE [dbo].[EmailAddress](
    [Id] [int] IDENTITY(1,1) NOT NULL,
 CONSTRAINT [PK_EmailAddress] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

Filling them with data will make the batch.sql.txt linked by @LonDat 's insert succeed and produce the same query plan as linked.

DECLARE @count int;
SET @count = 0;

WHILE @count < 100000
BEGIN 
    INSERT INTO EmailAddress DEFAULT VALUES;
    INSERT INTO EmailMessage DEFAULT VALUES;

    SET @count = @count + 1;
END

I first tried adding an identity column to the variable table with negative results -- the order of the variable table's identity ids was the order that the values were being outputted originally.

Based on my understanding of the query plan and testing, it appears the only valid solution is to add "ORDER BY [Id]" to "SELECT [Id] FROM @generated524" when the column is expected to be an identity column.

The guarantee talked about in the MSDN blog linked by @divega does apparently mean that the order of the items being inserted into the primary table is guaranteed, but the order of the items that are outputted to the table variable are based on undefined factors (in this case, FK constraint verification causes sorting 2x prior to being inserted into to the variable's table, as seen in the plan linked by @LonDat ).

@AndriySvyryd
Copy link
Member

I came up with a way to sort the results, but it might not be the most efficient query: http://stackoverflow.com/questions/34362390/return-inserted-rows-in-inserted-order-in-sql-server/34362391

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-bug
Projects
None yet
Development

No branches or pull requests

6 participants