dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.77k stars 3.18k forks source link

SQL Server: Switch IDENTITY_INSERT on (and then off) when explicit values are specified for an identity column #703

Open ajcvickers opened 10 years ago

ajcvickers commented 10 years ago

When using generated values the state manager will not generate a temporary value if a specific value was already set. The update pipeline should then handle inserting the specific value or throw an exception. This isn't currently implemented for Identity columns in SQL Server. (It is also an option that the update pipeline could somehow special case this situation using metadata information and still generate a new value, just as it did in the old stack, but this would be a break from the expected behavior for value generation.)

divega commented 10 years ago

I personally think it is fine to let the database server throw. At least the users will know that they are doing something wrong instead of having their specified values being ignored.

rowanmiller commented 8 years ago

With the implementation we landed on you will get an exception if you try to specify explicit values... but it is possible to make it work with a bit of extra code.

We could be smart about this in our provider and actually switch IDENTITY_INESRT on then off when we know that we're going to try and insert and explicit value.

ajcvickers commented 6 years ago

We should look at what was done for seed data and see if this can be generalized. However, even then it is not clear that the behavior should be switched on by default.

ajcvickers commented 6 years ago

See #11586 for an API proposal.

AndriySvyryd commented 5 years ago

Consider how this would interact with https://github.com/aspnet/EntityFrameworkCore/issues/13575

davisnw commented 4 years ago

I found this issue linked off of https://docs.microsoft.com/en-us/ef/core/saving/explicit-values-generated-properties#explicit-values-into-sql-server-identity-columns. This issue description also mentions that "state manager will not generate a temporary value".

Not having a full grasp of what "temporary value" means in the larger context, I'll just address automatically setting IDENTITY_INSERT ON/OFF.

While I can definitely see this being useful for seed data scenarios in testing (since Sql Server only allows IDENTITY_INSERT ON for one table at a time on the transaction/connection), I think that identity insert should not automatically be switched on/off by default.

In most application code I see (outside of seed data), explicitly assigning a value to a field that is mapped to an identity should be considered an application bug, and I would prefer Entity Framework to raise an exception if an attempt to set an explicit identity value is encountered.

In some cases, I see the identity field being used to obtain an ordering of when the application inserted records, e.g. because it is immune to system clock "jumps" when time synchronization occurs (understanding that "identity order" is not necessarily "transaction commit order")

For me it would be more useful to have explicit control over identity insert on a case-by-case basis, rather than a global configuration switch e.g. what was proposed in #13575 in that regard:

using (context.ChangeTracker.EnableExplicitKeys())
{
    context.Add(new Blog { Id = 77 });
}

Also there appears to be a bit of overlap with HasData, but that is a different use case. Notably, based on my limited testing in EfCore 2.2, it appears that HasData bypasses the normal db context hooks (that is I don't see e.g. SaveChangesAsync being called in the derived override), I would expect entities added under e.g. EnableExplicitKeys to go through all the normal DbContext hooks when SaveChanges / SaveChangesAsync is called.

IanKemp commented 4 years ago

The fact that this hasn't been implemented in over half a decade is mind-boggling.

DawidIzydor commented 3 years ago

Is there any chance this will be implemented in any forseeable future (ie. in the next 5 years)?

AndriySvyryd commented 3 years ago

The age of an issue has no impact on its priority, all currently open issues are expected to be implemented in the forseeable future.

Archomeda commented 1 year ago

3 years later, can someone explain the range of "the forseeable future" in context of EFCore?

roji commented 1 year ago

@Archomeda as always, we have tons of possible work items and a small number of engineers to do the work. At the end of the day there are just 24 votes on this, so it's really far down out request list and doesn't seem to affect many users.

Regardless, posting "when will this feature be implemented" doesn't help us prioritize, but rather takes up more of our time in writing answers.

Archomeda commented 1 year ago

@roji I came here via the link on this documentation page. I don't find it unreasonable to think that, if there's a link on the official pages mentioning "hey, we have a feature request for this", that it's something it's at least quite a recent request and not a dead one. Because you have to admit, if it's a 10 year old request, and, as you put it, doesn't seem to affect many other users, why is it even on there?

I'm sorry to have bothered anyone here for my obvious blunder by asking something I shouldn't have asked.

As I'm not a person who just leaves other people in the dark if I found an alternative solution to my problem: it seems that my use case didn't require identity columns in the first place and I could disable them in the context via .ValueGeneratedNever(). It seems that the internet put me on a wild goose chase of IDENTITY_INSERT instead for a couple of hours, because this method wasn't easy to find at all. Maybe this solution will help other people looking for an alternative for the same use case as mine.

IanKemp commented 1 year ago

Regardless, posting "when will this feature be implemented" doesn't help us prioritize, but rather takes up more of our time in writing answers.

This is quite possibly the most rude and insulting reply I've ever observed from a Microsoft employee on GitHub, but at least it makes very clear their actual attitude towards contributors, i.e. how much they (don't) value our contributions. Unless we're implementing features for Microsoft without being paid, so that Microsoft can make money off that unpaid work, we're not useful to them; we're just annoying mosquitoes to be fobbed off with excuses.

Good job on building community trust, Microsoft!

roji commented 1 year ago

I don't find it unreasonable to think that, if there's a link on the official pages mentioning "hey, we have a feature request for this", that it's something it's at least quite a recent request and not a dead one. Because you have to admit, if it's a 10 year old request, and, as you put it, doesn't seem to affect many other users, why is it even on there?

We indeed mention unimplemented features and limitations in our documentation. One reason for that is to make users aware that something isn't supported - as a heads up. The other is indeed to gather feedback from users who require this feature - but a simple up-vote on the top-most post is the standard and best way to let us know about it. For one thing, when we plan about what to implement in a given release, we look at highly-voted issues; comments posted down below saying "I need this" or "when will this be done" aren't visible and just get lost.

It's important to understand that if an issue is open, that means we've decided it makes sense for it to be done at some point; but that doesn't imply anything about when exactly that will happen. Our backlog is enormous, and it's entirely reasonable for an issue to remain open for years, since it receives very little votes or is out-prioritized by other features. We typically only close issues when we think they don't make sense for EF, or that there's some reason we'll never implement them.

Finally, @IanKemp and @Archomeda I can see how my answer may have been worded a bit strongly - my intent wasn't to insult anybody, and I apologize for that. First of all, note that there's no big "Microsoft" here - just a handful of engineers working hard on making EF Core better and engaging with the users. We get a constant stream of "how is this not yet done" and "when will this be implemented" messages; many of these are snarky, and some are downright aggressive. Regardless of their tone, these messages honestly do not help us prioritize features in any way - whereas up-votes definitely do. In other words, we're very much interested in user feedback - and get a lot of it - I'm just trying to explain how to do that in a way that's productive.