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

Guid [DatabaseGenerated(DatabaseGeneratedOption.Identity)] #14518

Closed
wstaelens opened this issue Jan 25, 2019 · 7 comments
Closed

Guid [DatabaseGenerated(DatabaseGeneratedOption.Identity)] #14518

wstaelens opened this issue Jan 25, 2019 · 7 comments

Comments

@wstaelens
Copy link

also similar:
https://stackoverflow.com/questions/47152056/ef-core-unique-indexed-auto-generated-guid-column-that-is-not-the-primary-ke

I have an asp.net core mvc 2.2.1 website using EF code first.
Model:

  public class Record
  {
    [Key]
    public int Id { get; set; }
    [DataType(DataType.Text)]
    public string Title { get; set; }
    [DataType(DataType.MultilineText)]
    public string Info { get; set; }
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public Guid UniqueId { get; set; }
}

In the controller in the Create method does not create and assign a new Guid, so i'm not manually adding Guids to the model on creation:

 if (ModelState.IsValid)
      {
        _Context.Add(record);
        await _Context.SaveChangesAsync();
        return RedirectToAction(nameof(Index));
      }

Create view:

A database operation failed while processing the request.
DbUpdateException: An error occurred while updating the entries. See the inner exception for details. 
SqlException: Cannot insert the value NULL into column 'UniqueId', table 'Foo.dbo.Record'; column does not allow nulls. INSERT fails. The statement has been terminated.
@ajcvickers
Copy link
Member

Note for triage. This should not attempt to insert null. However, it's not clear to me whether

 modelBuilder
            .Entity<Record>()
            .Property(e => e.UniqueId)
            .HasDefaultValueSql("newid()");

should be needed as well. On the one hand, this explicitly sets the property store-generated, which is necessary since there is no value converter. On the other hand, if ValueGenerated.OnAdd is specified and there is no value generator for the property, then maybe it must be store generated and so we shouldn't require HasDefaultValueSql.

@wstaelens
Copy link
Author

If I add the modelbuilder code with "newid()" to get it working. But what is the point of using dataannotations and adding [DatabaseGenerated(DatabaseGeneratedOption.Identity)] to a property (which is not the [Key]) if doesn't work and doesn't create a new Guid on insert?

@ajcvickers
Copy link
Member

@wstaelens This is a bug: it should treat the column as computed. But also see #14532

@ajcvickers ajcvickers added this to the 3.0.0 milestone Jan 28, 2019
@NetTecture
Copy link

It actually is a bug. NewId() is used for identifiers. Identity in SQL Server has a different meaning (i.e. int column auto counting). The way I did that in the last years was

  • Option Computed
  • Newid () as default value for insert

Alternatively actually handle it in code - the guid identity is one element that is nice to actually generate on the client and then use up to the server storage. That is sort of how it is meant to be used, the whole concept of a guid is that it CAN be generated on the client.

@zp978
Copy link

zp978 commented Mar 13, 2019

modelBuilder
.Entity()
.Property(e => e.UniqueId)
.HasDefaultValueSql("newid()");

if I use .HasDefaultValueSql("newid()"); , this is removed and returned back to .ValueGeneratedNever(); when scaffold command is run, what would be the correct approach to preserve .HasDefaultValueSql("newid()"); when Scafold is run in the future.

@ajcvickers
Copy link
Member

Needs some more investigation.

@AndriySvyryd
Copy link
Member

This is the command that we send

INSERT INTO [Records] ([Info], [Title])
VALUES (@p0, @p1);

Since the column is not setup to generate values SQL Server throws.

@AndriySvyryd AndriySvyryd removed their assignment Jul 24, 2020
@AndriySvyryd AndriySvyryd removed this from the 5.0.0 milestone Jul 24, 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

5 participants