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

Update command order with computed columns and unique constraints #33023

Closed
JustusGreiberORGADATA opened this issue Feb 7, 2024 · 2 comments · Fixed by #34626
Closed

Update command order with computed columns and unique constraints #33023

JustusGreiberORGADATA opened this issue Feb 7, 2024 · 2 comments · Fixed by #34626
Labels
area-save-changes closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression type-bug
Milestone

Comments

@JustusGreiberORGADATA
Copy link

JustusGreiberORGADATA commented Feb 7, 2024

Ask a question

EF Core will sort UPDATE commands in the method CommandBatchPreparer.TopologicalSort to make sure that unique constraints of the store are not violated (while in a transaction).

EF Core can do this, because it knows which columns the constraints apply to and which value changes occurred.

Things get trickier if you have a computed column with a unique constraint that depends on a regular column. EF Core by default can't know how the value in the computed column is related to the value in the regular column and therefore can't generate the correct topological sort to not violate the stores constraints.

Up until EF Core 6.0 we used a little "trick" to make EF Core aware of the data dependency: We created a property setter for the generated column, that told EF Core the relation between the columns. If you were to have a generated column like this:

modelBuilder.Entity<TestEntity>()
    .Property(e => e.IsPrimaryNormalized)
    .HasComputedColumnSql($"IIF(IsPrimary, true, null)", stored: true);

modelBuilder.Entity<TestEntity>()
    .HasIndex(e => new { e.UserId, e.IsPrimaryNormalized })
    .IsUnique();

We would create the following properties inside the entity:

public bool IsPrimary { get; set; }
public bool? IsPrimaryNormalized { get => IsPrimary ? true : null; private set { } }

This worked great - the update statements were always sorted correctly when we modified IsPrimary.

Now we are looking into upgrading to EF Core 7 or EF Core 8 and discovered that this "trick" no longer works. I am aware though that using properties like this to represent this data dependency might have never been part of the official feature set (this is why I call it a "trick"). This therefore might not really be a breaking change.

This is why I came here to ask: Is there an official story for making EF Core aware of the expected value of computed columns or the order in which update statements need to be executed? Is the above supposed to work?
Or do I just have to bite the bullet and call SaveChanges() twice and wrap the whole thing in an explicit transaction? (This would likely make the code more confusing for people reading it for the first time.)

Include your code

Question is already pretty long, so I put a full setup in this gist:

https://gist.github.com/JustusGreiberORGADATA/4f4529c282cc0abd379bb3871c11063d#file-program-cs

If this is a problem, just ping me and I will add it in a comment.

Include provider and version information

EF Core version: 6. Trying to update to 7 or 8.
Database provider: My first tests have this issue with Microsoft.EntityFrameworkCore.Sqlite. Later I also want this to work in MariaDB.
Target framework: .NET 7 for EF Core 7. .NET 8 for EF Core 8.
Operating system: Windows
IDE: Visual Studio 2022 17.8.6

@ajcvickers
Copy link
Member

/cc @AndriySvyryd

@AndriySvyryd
Copy link
Member

I don't think there's a better workaround for this currently, but we should be able to make this work again.

@ajcvickers ajcvickers added this to the 9.0.0 milestone Feb 9, 2024
@AndriySvyryd AndriySvyryd self-assigned this Apr 26, 2024
@AndriySvyryd AndriySvyryd removed their assignment Sep 6, 2024
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-save-changes closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants