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

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

Open
ajcvickers opened this Issue Sep 25, 2014 · 4 comments

Comments

Projects
None yet
4 participants
@ajcvickers
Member

ajcvickers commented Sep 25, 2014

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

This comment has been minimized.

Member

divega commented Sep 26, 2014

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

This comment has been minimized.

Member

rowanmiller commented Dec 3, 2015

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.

@rowanmiller rowanmiller changed the title from Support explicitly set values for Identity columns in SQL Server update pipeline to SQL Server: Switch IDENTITY_INSERT on (and then off) when explicit values are specified for an identity column Dec 3, 2015

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Feb 16, 2018

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

This comment has been minimized.

Member

ajcvickers commented Apr 7, 2018

See #11586 for an API proposal.

nfaugout added a commit to LuccaSA/RestDrivenDomain that referenced this issue Oct 3, 2018

REF Tests
Tests run under a disposable localdb database, with a GUID name. This ensure that SQL constraints will be respected.
Tests run in 24 sec now.
User now has a Guid identity column in order to avoir IDENTITY_INSERT ON/OFF on localdb database (cf aspnet/EntityFrameworkCore#703)
I have used a TestFixture to encapsulate the creation/deletion of the database
NB : Domain tests could be converted to use InMemoryStorage for faster run

@nfaugout nfaugout referenced this issue Oct 3, 2018

Merged

REF Tests #174

nfaugout added a commit to LuccaSA/RestDrivenDomain that referenced this issue Oct 3, 2018

REF Tests
Tests run under a disposable localdb database, with a GUID name. This ensure that SQL constraints will be respected.
Tests run in 24 sec now.
User now has a Guid identity column in order to avoir IDENTITY_INSERT ON/OFF on localdb database (cf aspnet/EntityFrameworkCore#703)
I have used a TestFixture to encapsulate the creation/deletion of the database
NB : Domain tests could be converted to use InMemoryStorage for faster run

nfaugout added a commit to LuccaSA/RestDrivenDomain that referenced this issue Oct 3, 2018

REF Tests
Tests run under a disposable localdb database, with a GUID name. This ensure that SQL constraints will be respected.
Tests run in 24 sec now.
User now has a Guid identity column in order to avoir IDENTITY_INSERT ON/OFF on localdb database (cf aspnet/EntityFrameworkCore#703)
I have used a TestFixture to encapsulate the creation/deletion of the database
NB : Domain tests could be converted to use InMemoryStorage for faster run
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment