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

Error in SequentialGuidGenerator #20527

Closed
peroyhav opened this issue Apr 4, 2020 · 7 comments
Closed

Error in SequentialGuidGenerator #20527

peroyhav opened this issue Apr 4, 2020 · 7 comments

Comments

@peroyhav
Copy link

peroyhav commented Apr 4, 2020

The SequentialGuidGenerator states it generates a new Guid in the same way as SQL server's NEWSEQUENTIALID(), however, since a new GUID is generated upon every insert, this does not generate the GUIDS in the same maner.

Steps to reproduce

Insert multiple entities and compare the GUIDs of the newly created entities, they do not only differ in an incremental manner.

Further technical details

The SequentialGuidValueGenerator should according to the documentation produce GUIDs in somewhat the same manner as the SQL server equivalent

To produce an example for testing on SQL server:

In order to correct, the bytes of the initially generated GUID should be stored in a field and used instead of generating a new Guid each time, in addition, if the GUID's shall be similar to a sequential GUID, the correct bytes should be updated, in the same manner as for SQL server.

e.g.

DECLARE @tbl TABLE(ID UNIQUEIDENTIFIER UNIQUE DEFAULT(NEWSEQUENTIALID()))

INSERT INTO @tbl VALUES(DEFAULT),(DEFAULT),(DEFAULT),(DEFAULT)
...
INSERT INTO @tbl VALUES(DEFAULT),(DEFAULT),(DEFAULT),(DEFAULT)

SELECT * FROM @tbl
@ajcvickers
Copy link
Member

ajcvickers commented Apr 6, 2020

@peroyhav Thanks for reporting this and for sending the PR. It looks like one of the same issues discussed on #11391 #19124.

@roji Can you look at this issue together with #11391 #19124 and the PR #20528 and determine if we need to do anything else as well?

@peroyhav
Copy link
Author

peroyhav commented Apr 6, 2020

@ajcvickers I can't see the resemblance with SQL server and IIS Express here, the error in #11391 is an error when using EF core in IIS Express, while the error I reported, and proposed a solution for through PR #20528 is an issue with the way Sequential GUIDs were generated.

@ajcvickers
Copy link
Member

@peroyhav Linked the wrong issue. 🤦‍♂ Should be #19124.

@peroyhav
Copy link
Author

peroyhav commented Apr 6, 2020

@ajcvickers Ok, thanks, It looks like this will solve parts of #19124. I'm not sure what clock value is used to seed SQL server, so I've used DateTime.UtcNow, the incremental part is therefore somewhat different from that of SQL server, In adition SQL server seems to have a constant node-id per server instance, and counts , the version I implemented, will have different node-id per instance. to ensure the same behavior, the count values should probably be static across instances, this would then require higher care to ensure thread safety. If I'm to do more work on this; are there any standards that should be taken into consideration simultaneously?

@peroyhav
Copy link
Author

peroyhav commented Apr 6, 2020

This Issue looks a lot like #19124 I see that I have omitted Value in the class name when searching for Github issues, so it didn't pop up when I searched.

@roji
Copy link
Member

roji commented Apr 13, 2020

Duplicate of #19124

@roji roji marked this as a duplicate of #19124 Apr 13, 2020
@roji
Copy link
Member

roji commented Apr 13, 2020

See valuable conversation and analysis in #20528 (thanks for doing that @peroyhav).

@roji roji closed this as completed Apr 13, 2020
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants