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

Explicitly configuring value generation: add [Timestamp] #3165

Closed
Rick-Anderson opened this issue Mar 20, 2021 · 6 comments
Closed

Explicitly configuring value generation: add [Timestamp] #3165

Rick-Anderson opened this issue Mar 20, 2021 · 6 comments

Comments

@Rick-Anderson
Copy link
Contributor

Given that [Timestamp] is the recommended approach for concurrency tokens, that should be added to the Explicitly configuring value generation section.

In the article:

For example, on SQL Server, when a GUID property is configured as value generated on add, the provider automatically performs value generation client-side, using an algorithm to generate optimal sequential GUID values. However, specifying ValueGeneratedOnAdd() on a DateTime property will have no effect (see the section below for DateTime value generation).

Does that only work on add? What about ValueGeneratedOnAddOrUpdate? If it works on ValueGeneratedOnAddOrUpdate, is GUID a good alternative to using [Timestamp]?

Similarly, byte[] properties that are configured as generated on add or update and marked as concurrency tokens are set up with the rowversion data type, so that values are automatically generated in the database. However, specifying ValueGeneratedOnAddOrUpdate() will again have no effect.

With the following in a model:

        [Timestamp]
        public byte[] RowVersion { get; set; }

Why is the following code in the BuildModel method generated for the preceding code?

 b.Property<byte[]>("ConcurrencyToken")
     .IsConcurrencyToken()
     .ValueGeneratedOnAddOrUpdate()
     .HasColumnType("rowversion");

The [SQLite and SQL Server concurrency versions]
(https://github.com/dotnet/AspNetCore.Docs/blob/fbea9dcb0b62f6a238498361ecd1e69358224718/aspnetcore/data/ef-rp/intro/samples/cu50/Pages/Departments/Edit.cshtml.cs) in dotnet/AspNetCore.Docs#21576 in have only the following differences:

+ using System;    // For GUID on SQLite version

+ departmentToUpdate.ConcurrencyToken = Guid.NewGuid();

 _context.Entry(departmentToUpdate)
    .Property(d => d.ConcurrencyToken).OriginalValue = Department.ConcurrencyToken;

- Department.ConcurrencyToken = (byte[])dbValues.ConcurrencyToken;
+ Department.ConcurrencyToken = dbValues.ConcurrencyToken;

It's tempting to use GUID for both, but [Timestamp] is the recommended approach for SQL Server.

Please assign to me if appropriate and issue is accepted. cc @JeremyLikness @AndriySvyryd


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@roji
Copy link
Member

roji commented Mar 20, 2021

Given that [Timestamp] is the recommended approach for concurrency tokens, that should be added to the Explicitly configuring value generation section.

There's this sentence right below the SQL Server Guid generation one which you cited (though it's more of an example on how value generation works, and indeed doesn't mention the [Timestamp] attribute specifically):

Similarly, byte[] properties that are configured as generated on add or update and marked as concurrency tokens are set up with the rowversion data type, so that values are automatically generated in the database. However, specifying ValueGeneratedOnAddOrUpdate() will again have no effect.

At least in my mind, we tend to distinguish between "simple" value generation and automatically-generated concurrency tokens such as rowversion. The distinction isn't 100% rigorous or anything, but a meaningless concurrency token which changes on each update pretty much cannot be used for anything other than concurrency checks; Guid generation on add is useful in other ways, i.e. to assign primary keys. This is why we have two separate pages for value generation and for concurrency tokens. We can definitely link the concurrency token page from the value generation page, though I think it still makes sense to keep most concurrency token information in its separate page.

For example, on SQL Server, when a GUID property is configured as value generated on add, the provider automatically performs value generation client-side, using an algorithm to generate optimal sequential GUID values. However, specifying ValueGeneratedOnAdd() on a DateTime property will have no effect (see the section below for DateTime value generation).

Does that only work on add? What about ValueGeneratedOnAddOrUpdate? If it works on ValueGeneratedOnAddOrUpdate, is GUID a good alternative to using [Timestamp]?

No, client-side value generation (with a ValueGenerator such as SequentialGuidValueGenerator) work only on add (dotnet/efcore#6999 tracks this). Client-side value generation is generally not documented enough, I was planning to work on this in #3057 - I can do that now along with the changes you're working on.

Why is the following code in the BuildModel method generated for the preceding code?

I'm not sure exactly what you're asking... EF Core has a convention which identifies the [Timestamp] attribute and configures the property as both ValueGeneratedOnAdd and ConcurrencyToken; the SQL Server type mapper then selects the rowversion type for byte[] properties which have ValueGeneratedOnAdd and ConcurrencyToken. So the equivalent Fluent API configuration for [Timestamp] is indeed that longer and more explicit fragment you posted.

Re SQL Server vs. Sqlite... When your database supports a data type that gets automatically generated on each update (such as rowversion on SQL Server), that's usually the simplest and recommended way to do optimistic concurrency. Sqlite doesn't have this, so it's the user's responsibility to generate manage Guids on each update. When we do dotnet/efcore#6999, things would become easier for Sqlite (and other databases with a similar situation).

@Rick-Anderson
Copy link
Contributor Author

Similarly, byte[] properties that are configured as generated on add or update and marked as concurrency tokens are set up with the rowversion data type, so that values are automatically generated in the database. However, specifying ValueGeneratedOnAddOrUpdate() will again have no effect.

With the following in a model:

        [Timestamp]
        public byte[] RowVersion { get; set; }

Why is the following code in the BuildModel method generated for the preceding code?

 b.Property<byte[]>("ConcurrencyToken")
     .IsConcurrencyToken()
     .ValueGeneratedOnAddOrUpdate()
     .HasColumnType("rowversion");

So the question is about

However, specifying ValueGeneratedOnAddOrUpdate() will again have no effect.

Then why is .ValueGeneratedOnAddOrUpdate() generated in the preceding b.Property snippet?

@roji
Copy link
Member

roji commented Mar 20, 2021

I see what you mean now - yeah, that sentence is incorrect, it should be ValueGeneratedOnAdd - this has no effect on a byte[] property.

@roji
Copy link
Member

roji commented Oct 21, 2022

This was already fixed - we now say "specifying ValueGeneratedOnAdd has no effect", instead of ValueGeneratedOnAddOrUpdate.

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Oct 21, 2022
@roji
Copy link
Member

roji commented Oct 21, 2022

This was already fixed - we now say "specifying ValueGeneratedOnAdd has no effect", instead of ValueGeneratedOnAddOrUpdate.

1 similar comment
@roji
Copy link
Member

roji commented Oct 21, 2022

This was already fixed - we now say "specifying ValueGeneratedOnAdd has no effect", instead of ValueGeneratedOnAddOrUpdate.

@roji roji closed this as completed Oct 21, 2022
@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Oct 21, 2022
@roji roji removed this from the 7.0.0 milestone Oct 21, 2022
@roji roji removed their assignment Oct 21, 2022
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

No branches or pull requests

4 participants