-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
CascadeDelete in "appliction/domain layer" instead of Database #23188
Comments
Related (possible dup): #13146 |
@John0King Some comments:
I think the only thing missing from EF Core in what you describe is the ability to delete entities without loading them into the context. This is tracked by #795. |
yes, I can do , but the number of table grows by time, and if I want to delete the FK then I had to delete them every time when I add a migration (including I previously deleted)
I already show you a demo above, that without a FK constraints , it's possible that the data is incorrect like the soft delete example public class Article
{
public int AuthorId { get;set;}
public Author Author{ get;set;} // application layer FK
}
public class Author
{
public int Id {get;set;}
public string Name{get;set;}
}
// add data
var author = new Author{ } // id=1
var artcle = new Article{ AuthorId = 1 }
db.Add(author)
db.Add(artcle)
db.Savechanges();
db.Remove(author) // we remove a author , but won't remove the article
// don't be surprise there, if your search stackoverflow for `should I create a FK` then , most answer will tell you without a FK constraints , then there will be lots of garbage data in database
db.Savechanges()
// paged query
var query = db.Articles.Inlcude(x=>x.Author);
// var result = query.ToPagedResult(pageIndex,pageSize); // for example 1, 10
var count = query.Count(); // 1
var result = query.ToList(); // 0
result on page:
|
A full solution here could also mean EF Core performing a bulk delete (or set null) on the database when a principal is deleted, to mimic the database behavor for cascading deletes. |
@roji yes, we do need bulk delete, and the fix for |
@roji Historically, that has always been part of the bulk update issue--there's discussion from Diego on #795. We could track it separately, but I think the feeling has been that if we do bulk updates, then using them for cascades is an important part of that. @John0King In your example you create an optional relationship with a nullable FK property, in shadow state. EF does not by default configure optional relationships to cascade delete. If you made the relationship required or explicitly configured it to have cascade delete, then in your example both entities would be deleted since at the time you call SaveChanges they are both tracked. |
@ajcvickers thanks for your explain. but I think you still miss my point. let me explain. first, I define db.Articles.Incluce(x=>x.Author).ToList() SELECT a.* FROM Articles as a INNER JOIN Author as u ON a.AuthorId = u.Id but when I query using : db.Articles.Incluce(x=>x.Author).Count() SELECT Count(a.*) FROM Articles as a you can see , that the Count() lost the another example is using soft Delete; public partial class Author
{
public bool IsDeleted {get;set;}
}
builder.Entity<Author>(b=> b.HasQueryFileters(a=>a.IsDeleted == false)) and then imaging the Author is been soft deleted db.Articles.Incluce(x=>x.Author).ToList() SELECT a.* FROM Articles as a INNER JOIN ( SELECT * FROM Author WHERE IsDeleted = 0) as u ON a.AuthorId = u.Id but when I query using : db.Articles.Incluce(x=>x.Author).Count() SELECT Count(a.*) FROM Articles as a and in this example the error is occur even when the database has the FK constraint. |
Why is the inner join needed to get the count? In fact, would this not be the wrong result? Include will never effect the count of entities returned since, for example, there are the same number of Articles regardless which of them have Authors. You say that you create a "non-nullable FK property," but it sounds like there are null values in the column. If a column contains null values, then it must be configured as nullable in the EF model. Otherwise you will at best get exceptions, and at worst get bad data. |
@ajcvickers I think you still in "strict data + should" mode to think the data relationship.
In strict model (eg. Database constraint enabled)define entity public class Blog
{
public int Id { get;set; }
public string Name { get;set; }
public bool IsDeleted{ get; set; } // default is false
}
public class Post
{
public int Id { get; set; }
public string PostName { get; set; }
// with goal 3 , so I define that the BlogId is requred FK eg. Noe-nullable-FK
public int BlogId { get; set; }
public Blog Blog{ get;set;}
} configure DataContext public class AppDb :DbContext
{
public DbSet<Blog> Blogs{get;set;}
public DbSet<Post> Posts{ get;set;}
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
base.OnModelCreating(modelBuilder);
modelBuilder.Entity<Blog>(builder=>
{
builder.HasQueryFilter(x=>x.IsDeleted == false); // we set this filter so we well won't see any soft deleted
});
}
} and run using static void Main(string[] args)
{
SeedData();
var db = new AppDb();
var query = db.Posts.Include(x => x.Blog);
var count = query.Count();
var result = query.ToList();
Console.WriteLine($"there are counted [{count}] posts");
Console.WriteLine($"there are actually [{result.Count}] posts ");
foreach (var p in result)
{
Console.WriteLine($"\tname:[{p.PostName }] blogId: [{p.BlogId}] blog is deleted: { p.Blog?.IsDeleted ?? true } ");
}
} and the result
In lax mode (eg. without Database constraint )there can be no soflt deleted so the result is the same, because we do everything in our "application layer" instead of |
@ajcvickers I made a example repo and you can try to run that https://github.com/John0King/EfCoreFKDemo |
@smitpatel Why does the query filter on Blog impact the number of Posts returned here? |
@John0King So you should see a warning in the logs about adding a query filter to Blogs but not Posts. Otherwise your model doesn't prevent a non-deleted Post from existing for a deleted Blog. I think the fundamental problem here is that using a query filter is not a full solution to soft delete (see #19695). You can use query filters to get some of the behavior of soft deletes, but you have to be careful not to create invalid states like this in your model. (In other words, EF has no idea what the |
Given the model configuration without soft delete scenario. When soft delete is enabled, as Arthur pointed out, your data is inconsistent with the model
The issue with filters and joins has been extensively discussed in #11691 |
@smitpatel @ajcvickers Thank for your explain, but I don't agree with you. first , the database relationship with application is not one to one .
you can see for this demo , it's incorrect behavior now. Why we write a additional with my first point. a database can be use for a .Net applcation and a php application, and then the db is not having FK (not soft delete), and the data for Data:
Result :sense the data can also managed by php app , we can not assume the php will delete every relational data( a big application may have hundreds of those relationship) and when this issue(23188) been solved then in .net using EF Core we can not assume the data too, because some relationship may add later (lost relationship in first round of development). so my opinion is do not do "JOIN optimization" for ps: for a example of so doing the FK in applcation layer maybe the best option. |
Neither of the conclusions are actually correct. Putting all of EF Core, database constraints and query filters away.
Now important thing here is that navigation if loaded from database then the principal key on blog instance pointed by that navigation matches the foreign key property value on post instance. That is very basic definition of navigations. If you want to call this an assumption of EF Core then there is no other way for us to identify what each Blog navigation points to since navigations do not exists in database. You are free to have a Blog property on Posts and set it yourself to whatever and ignore it in EF model. Now let's introduce query filter on top of above.
In the essence, you have defined a filtered view of your database which is entirely incorrect for a .NET client where navigations exists. I would assume same issue would be in any other type of programming language, but I am not expert and not going to comment on it.
There is no "JOIN optimization" for "Include". Even if we do LEFT JOIN we cannot materialize results because FK value and navigation values are mismatching. Bottom line Define the behavior of your data in your app which actually makes sense. What happens to posts if associated blog is soft-deleted
Query is neither requiring FK constraint in database nor enforcing correctness of data in database. If you don't give us data which can be materialized then you will get error. I need to create client side objects in the end. |
I previously discuss in #21512 , and maybe I'm not clear my thought and that issue is closed. but I think it's important。
Current Cascade Options
Cascade
data been deleted by database setting(cascade setting) and to delete it inChangeTracker
for in memeory dataClientSetNull
delete it inChangeTracker
for in memeory data onlySetNull
set to null by database setting(cascade setting) and to set null inChangeTracker
for in memeory dataRestrict
throw for non-nullable foreign keyand database relation has limited, that limited the child operation of 3 level, you can not cascade more. and database is doing hard job on concurrent query, and by using foreign key relation is slower than normal table (with cascade is much slow)
see Alibaba 's Database Design Rule and alibaba's taobao.com and tianmao.com is "Industry benchmark/standard" for huge request.
translate :
and base on my know, today(maybe the past) the big Hold back of the .net web site performace is database (java has
sharding-jdbc
for sepreate table/database that can manage huge data and improve performace of data). and see cnblog's experience, this may not relate this problem, but it show's that the challenge of .net website is always the database , I have the same experience for my company website. with all this, I think we should do something if we have the change to can make database has less work to do and make database more faster.so, let's think how we can do in applcation layer.
builder.Entity<T>().WithoutForeignKey()
) and it's not important for now..HasForeignKey()
)Delete
Sql to delete it to databaseand fix the bug behavior for below
maybe I was remember wrong, in EF core 1.x , there is an article said that there is a JOIN optimizer that reduce the join if the result do not contains the second table field. but @smitpatel said there is not. but anyway the same query generate different SQL statement (the main query part) should not be a correct behavior. and consider that the
author
is been soft deleted with global filter, withoutJOIN (SELECT * FROM Author WHERE Authro.IsDeleted = false) as A ON Article.AuthorId = A.Id
the query is still incorrect.The text was updated successfully, but these errors were encountered: