String properties with indexes are compared case insensitively on SQL Server #32898
Comments
|
Note for triage: I am able to reproduce this; having a unique index defined on the string property appears to be a necessary condition. I expect this is causing the key comparer to be used somewhere it should not. int scenarioId;
using (var context = new SomeDbContext())
{
await context.Database.EnsureDeletedAsync();
await context.Database.EnsureCreatedAsync();
var entity = new Foo { Name = "UPPERlower" };
context.Add(entity);
await context.SaveChangesAsync();
scenarioId = entity.Id;
}
using (var context = new SomeDbContext())
{
var dbModel = await context.Set<Foo>().SingleAsync(s => s.Id == scenarioId);
dbModel.Name = dbModel.Name.ToUpperInvariant();
await context.SaveChangesAsync(); // No UPDATE statement issued in SQL
}
using (var context = new SomeDbContext())
{
var dbModel = await context.Set<Foo>().SingleAsync(s => s.Id == scenarioId);
Console.WriteLine(dbModel.Name);
}
public class SomeDbContext : DbContext
{
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder
.UseSqlServer(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow")
.LogTo(Console.WriteLine, LogLevel.Information)
.EnableSensitiveDataLogging();
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Foo>(b =>
{
b.HasIndex(x => x.Name).IsUnique();
});
}
}
public class Foo
{
public int Id { get; set; }
public required string Name { get; set; }
} |
|
@GeorgDangl I investigated this some more, and this is an intentional consequence of #27526, which also applies to indexes. You can get back the previous behavior by setting your own value comparer. For example: var comparer = new ValueComparer<string>(
(l, r) => string.Equals(l, r, StringComparison.Ordinal),
v => v.GetHashCode(),
v => v);
modelBuilder.Entity<Foo>(
b =>
{
b.HasIndex(x => x.Name).IsUnique();
b.Property(e => e.Name).Metadata.SetValueComparer(comparer);
});
}Note for triage: it's not clear that setting the collation should change this because we don't know whether the collation is case sensitive or insensitive. It's also not clear to me that we should have done this by default for index columns, but it's also not clear that we shouldn't. For now, I think we should document as a breaking change with the mitigation being the code above. |
|
Note from triage: filed #32546. |
|
Moved to docs to document as a breaking change. |
|
@ajcvickers, thank you for the detailed explanation! Sounds like it makes sense, and I think announcing it as a breaking change would be good. Maybe this could also be covered by an analyzer, since I can imagine there are many use cases for having unique indexes on case sensitive string columns. |
|
@ajcvickers Is there anything we can add to change this back by convention instead of needing to find all places and update with the own value converter overload? In our use case we have an id that is case insensitive by it's meaning but the value itself is important to be in the case that it is entered, example we want first created it as |
Fixes #32898 Note that we didn't change what we do in the update pipeline, which appears to already be using provider values.
ajcvickers
added
the
closed-fixed
Fixes #32898 Note that we didn't change what we do in the update pipeline, which appears to already be using provider values.
ajcvickers
changed the title
oldValue.ToUpperInvariant() on SQL Server does not result in an update with .NET 8Fixes #32898 Note that we didn't change what we do in the update pipeline, which appears to already be using provider values.
is this the only solution till today?, there are many classes to update and is not practicall. is there a proper solution? |
As the title states, we've had an endpoint in our ASP.NET Core Web API project covered by an integration test. The test itself was pretty simply, we're issuing a
PUTrequest to update some database model. The failing tests tried to set the name to all uppercase and all lowercase viastring.ToUpperInvariant()andstring.ToLowerInvariant(). The tests started failing after I did the update to .NET 8 with EF Core 8.Include your code
Here's a very minimal example from our code:
In the tests, we're running against MS SQL Server 2017 on Linux (in a Docker container on a tmpfs filesystem).
For the code block above, I'm getting the following log output. Which pretty clearly states that it saved zero changes.
I've tried the exact same code with another entity, an user from the ASP.NET Core Identity EF storage. There, I could change the username as expected to an all uppercase representation.
The model we're updating looks like this, the string property in question (
Name) is marked as required and has a max length of256:I couldn't find anything in the GitHub repo, except this issue which might seem like it's related, but I don't see anything in the PR that would affect the change tracker to not detect changes there. But, I'm also not very deep into the EF Core codebase:
#27526
Edit: I just took a look at the Debug view of the
ChangeTracker, and there I saw that it was tracking the entity via it'sIdproperty, and I saw the difference there:Also, when I set another value, e.g. just
Guid.NewGuid.ToString(), the change was saved correctly to the database.Include provider and version information
EF Core version:
8.0.0Database provider:
Microsoft.EntityFrameworkCore.SqlServerTarget framework:
.NET 6.0Operating system:
IDE:
Visual Studio 2022 17.8.0The text was updated successfully, but these errors were encountered: