Skip to content

fixed race condition causing incorrect IDs to be reported on INSERT #16

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

Merged
merged 1 commit into from
Dec 9, 2014

Conversation

bitmage
Copy link
Contributor

@bitmage bitmage commented Dec 8, 2014

When sending multiple INSERT operations to the MSSQL driver asynchronously, sometimes the IDs are incorrectly reported. I have observed 1) all the IDs being the same, 2) some IDs misreported, others correct. This is because MsSQL.prototype.create contains two SQL commands, the sum of which is not atomic. This change makes them atomic by wrapping them in a transaction.

Correct results are really important here - this was causing my foreign-key relationships to be configured incorrectly, which is a loss of data integrity.

I don't see a strong rationale for testing this, as it would be difficult to replicate, and would introduce indeterminacy into your test suite.

@slnode
Copy link

slnode commented Dec 8, 2014

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@bitmage
Copy link
Contributor Author

bitmage commented Dec 8, 2014

Yes, that looks much cleaner. I have updated the code - it will now produce SQL that looks like this:

INSERT INTO [dbo].[City] ([name],[type],[county])
OUTPUT INSERTED.id AS insertId
VALUES ((?),(?),(?));

Also note I am using the idName() function to calculate the ID field, so this should be compatible with customizations.

@bitmage
Copy link
Contributor Author

bitmage commented Dec 8, 2014

@raymondfeng Sorry, I think it deleted your comment when I rebased.

@raymondfeng
Copy link
Contributor

That's weird :-). Racing condition on github?

raymondfeng added a commit that referenced this pull request Dec 9, 2014
fixed race condition causing incorrect IDs to be reported on INSERT
@raymondfeng raymondfeng merged commit cee6636 into loopbackio:master Dec 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants