-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[release/10.0] Apply pending selector before ExecuteUpdate #37262
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
Conversation
|
Detailed analysis, for future reference: In EF 9.0, navigation expansion produced the following tree: ...
DbSet<BookingLeg>()
.Join(
inner: DbSet<Booking>(),
outerKeySelector: b1 => EF.Property<int?>(b1, "BookingId"),
innerKeySelector: b2 => EF.Property<int?>(b2, "Id"),
resultSelector: (o, i) => new TransparentIdentifier<BookingLeg, Booking>(
Outer = o,
Inner = i
))
.SelectMany(
collectionSelector: b1 => DbSet<GateEvent>()
.Where(g0 => EF.Property<int?>(b1.Inner, "Id") != null && object.Equals(
objA: (object)EF.Property<int?>(b1.Inner, "Id"),
objB: (object)EF.Property<int?>(g0, "BookingId")))
.Where(g0 => g0.IsInGate),
resultSelector: (b1, c) => new TransparentIdentifier<TransparentIdentifier<BookingLeg, Booking>, GateEvent>(
Outer = b1,
Inner = c
))
.Select(ti0 => ti0.Inner)
.ExecuteUpdate(str => str
.SetProperty<DateTime?>(
propertyExpression: p => p.Confirmed,
valueExpression: (DateTime?)DateTime.Now)
.SetProperty<string>(
propertyExpression: p => p.ConfirmedBy,
valueExpression: "Test"))In 10.0, thanks to #36723, nav expansion now properly handles ExecuteUpdate like a 1st-class LINQ operator - previously it was handled as an unknown method. Unfortunately, with the current implementation, that also means that any pending selector (the Select about just before ExecuteUpdate) flows into the ExecuteUpdate setters; so we get the following tree: DbSet<BookingLeg>()
.Join(
inner: DbSet<Booking>(),
outerKeySelector: b => EF.Property<int?>(b, "BookingId"),
innerKeySelector: b0 => EF.Property<int?>(b0, "Id"),
resultSelector: (o, i) => new TransparentIdentifier<BookingLeg, Booking>(
Outer = o,
Inner = i
))
.SelectMany(
collectionSelector: b => DbSet<GateEvent>()
.Where(g => EF.Property<int?>(b.Inner, "Id") != null && object.Equals(
objA: (object)EF.Property<int?>(b.Inner, "Id"),
objB: (object)EF.Property<int?>(g, "BookingId")))
.Where(g => g.IsInGate),
resultSelector: (b, c) => new TransparentIdentifier<TransparentIdentifier<BookingLeg, Booking>, GateEvent>(
Outer = b,
Inner = c
))
.ExecuteUpdate(new Tuple<Delegate, object>[]
{
new Tuple<Delegate, object>(
ti => ti.Inner.Confirmed,
(object)@p
),
new Tuple<Delegate, object>(
ti => ti.Inner.ConfirmedBy,
@p0
)
})While this is technically OK, the more complicated setters cause a failure in later processing. Specifically, the logic to push down complex ExecuteUpdate into a PK-based subquery assumes simple property selectors, where the lambda parameters are all referentially identical across the setter lambdas. but with the pending selector flowing in, we get full MemberExpressions ( This PR applies any pending selector before processing the ExecuteUpdate setters, adding back the Select() before ExecuteUpdate() and simplifying the setters again. This bug affects cases where:
|
39d8caf to
f72552b
Compare
artl93
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Customer reported regression. Simple change.
Closes #37247
Description
EF 10 brought various improvements to ExecuteUpdate; among them was #36723, which brought full processing of ExecuteUpdate in our navigation expansion, like any other LINQ operator (previously it was processed as an unknown method, and navigations inside the setter lambdas were not expanded). Unfortunately, with the current implementation, that also means that any pending selector (the Select about just before ExecuteUpdate) flows into the ExecuteUpdate setters. The more complicated setters caused a failure in later processing, specifically in PK-based subquery handling, for natively-unsupported LINQ operators.
See below for an in-depth analysis, for reference.
Customer impact
This bug affects cases where:
Such invocations of ExecuteUpdate throw an exception.
How found
Customer reported on 10.0.0
Regression
Yes.
Testing
Added.
Risk
Low: short, targeted fix to code already significantly modified in 10.0. Quirk added.