zzzprojects / EntityFramework-Plus

Entity Framework Plus extends your DbContext with must-haves features: Include Filter, Auditing, Caching, Query Future, Batch Delete, Batch Update, and more
https://entityframework-plus.net/
MIT License
2.27k stars 318 forks source link

BulkMerge(Async) with IncludeGraphOperationBuilder IgnoreOnMergeUpdate not being honored #797

Closed tsanton closed 4 months ago

tsanton commented 7 months ago

1. Description

I'm having some issues with the IncludeGraphOperationBuilder. I have a parent with three children, where I want to upsert the parent, upsert child1, insert child2 and conditionally insert child3.

In short my merge config looks as such:

options.IncludeGraphOperationBuilder = bulk =>
    {
        switch (bulk)
        {
            case BulkOperation<ZEntity> z:
                z.InsertIfNotExists = true;
                z.IgnoreOnMergeUpdateExpression = c => new { c.Tid, c.Fid, c.Id, c.K };
                z.MergeMatchedAndOneNotConditionExpression = c => new { c.e, c.f };
                break;
            case BulkOperation<UEntity> u:
                u.InsertIfNotExists = true;
                u.IgnoreOnMergeUpdateExpression = c => new {c.Id, c.UAT, c.UAS, c.UT };
                u.MergeMatchedAndOneNotConditionExpression = c => new { c.R, c.RB };
                break;
            case BulkOperation<UAEntity> ua:
                ua.IgnoreOnMergeUpdate = true;
                ua.InsertIfNotExists = true;
                break;
            case BulkOperation<USEntity> us:
                us.IgnoreOnMergeUpdate = true;
                us.InsertIfNotExists = true;
                us.ColumnPrimaryKeyExpression = c => new { c.Tid, c.Id, c.S };
                break;
            default: throw new NotImplementedException()
        }
    };

Here is what I want to achieve:

As of now the IgnoreOnMergeUpdate is not being honored. I still see the UPDATE statements generated for UA and US. I know I can achieve the desired outcome by just IgnoreOnMergeUpdateExpression all properties in the entity, but I am somewhat hoping for an easier fix by simply ignoring the update (making it a flat conditional insert) for the sake of performance.

Haven't tested the other way around, but I would hope InsertIfNotExists = false would remove the insert part in cases where you only want updates but not inserts in a somewhat complex setup.

Further technical details

JonathanMagnan commented 7 months ago

Hello @tsanton ,

Thank you for reporting. My developer will look into it tomorrow morning.

Best Regards,

Jon

JonathanMagnan commented 7 months ago

Hello @tsanton ,

Thank you again for reporting.

We currently have the right behavior for SQL Server but indeed as you reported, it looks like we have a bug with PostgreSQL.

We will look more and try to fix it.

Best Regards,

Jon

tsanton commented 7 months ago

Many thanks @JonathanMagnan! Hope to hear back from you here when the issue is closed or a resolution timeline exist :)

Have a great weekend when that time comes; speak soon!

/T

JonathanMagnan commented 6 months ago

Hello @tsanton ,

A new version has been deployed today.

Could you try it and confirm us that the issue has been fixed correctly?

Best Regards,

Jon

tsanton commented 6 months ago

Hi @JonathanMagnan!

Just tested it and I can confirm that the {opt => opt.IgnoreOnMergeUpdate = true; opt.InsertIfNotExists = true} generates only an insert statement. This is according to expectations and solves my "performance" issue. Many thanks!

Just for giggles I tried the inverse: {opt => opt.IgnoreOnMergeUpdate = false; opt.InsertIfNotExists = false}. In this case I expect only the update statement to be generated. From what I'm seeing this is not the case. It seems to be that opt.InsertIfNotExists = false is not being honored because I'm stilling seeing both an update and an insert for my entity.

My issue is solved, but I don't think that the config is behaving exactly as expected.

I still would like to say many thanks for the expedience you show in regard to replying, following up, solving issues!

/T

JonathanMagnan commented 6 months ago

Hello @tsanton ,

My bad, I forgot to tell you that the option InsertIfNotExists only works with BulkInsert

So, from what I understand, you would like an option IgnoreOnMergeInsert that behaves like the IgnoreOnMergeUpdate but for the INSERT statement.

I will ask my developer to look at it. Unfortunately, it will not be available before the end of May as we are currently pausing release for 3 weeks during my vacation.

Best Regards,

Jon

tsanton commented 6 months ago

@JonathanMagnan I see, copy that!

As of right now this isn't a blocker/issue as UpdateNotInsert is a not a part of my current use pattern. On the other hand, I can imagine scenarios where one would like only to update and not to insert (in the context of a "complex" merge with multiple child entities). If I am free to chose I would vote "do it".

I do think this leans towards the "nice to have" side, rather than "must have". In my opinion it would enhance the package simply because it turns Merge into a powerhouse, capable of any constellation of UpdateInsert(Delete?) for any and all entities. In combination with transactions it certainly lets me execute complex operations with high fidelity.

What I would suggest, in combination with this implementation, is to enhance the Merge documentation with examples where a complex Insert/Update case much like my scenario above with multiple child entities is well described with config in code and behaviour output as SQL. The functionality of your Merge is second to none, but I do think it's difficult for people without "hardcore" SQL-backgrounds to fully grasp how powerful, functional and flexible the method is.

I'll leave you guys be for the next three weeks then and get back to bothering you come June :)

Enjoy your vacation!

/T

JonathanMagnan commented 6 months ago

I do think this leans towards the "nice to have" side, rather than "must have"

I 100% agree with you. That's something I will make sure it happens. I do not believe it will be really hard on our side but surely ask us a few changes.

Enjoy your vacation!

Thank ;) I will sure do!!!

JonathanMagnan commented 5 months ago

Hello @tsanton ,

A new version is now available: https://github.com/zzzprojects/EntityFramework-Extensions/releases/tag/8.102.3

As promised, we added support to the new option IgnoreOnMergeInsert to SQL Server and PostgreSQL

Could you try it and let us know if everything is working as expected?

Best Regards,

Jon

tsanton commented 5 months ago

Give me until the end of the week and I'll give it a go! :)

tsanton commented 4 months ago

Hi Jon, and again: ended up in the backlog and I forgot to test it. Don't have any use patterns for this just yet, but I "broke" a test of mine by setting IgnoreOnMergeInsert = true which confirm that it works. I'll keep it in mind; many thanks for the "nice to have" implementation, though it sure in this case was the IgnoreOnMergeUpdate giving me idempotent "insert only" behaviour in a complex merge with child identities that saved the day:)

Feel free to close the issue and as always: 10/10!

/T