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

Fixes around IncludeExpression for ExecuteUpdate/Delete #30571

Merged
merged 2 commits into from
Mar 25, 2023

Conversation

roji
Copy link
Member

@roji roji commented Mar 24, 2023

Fixes #30528
Fixes #30572

maumar
maumar previously approved these changes Mar 24, 2023
@roji
Copy link
Member Author

roji commented Mar 24, 2023

@maumar I pushed another commit here in order to also fix #30572, which is related; basically is to prune all IncludeExpressions before ExecuteUpdate/Delete, since they have no meaning in that context (nothing is coming back from the database) and they interfere with translation.

This works well for SQL Server, but the new test Delete_entity_with_auto_include fails for SQLite; since SQLite doesn't support JOINs directly, we use a different path and transform to subquery with entity equality (code). There's a weird exception happening in this path, which looks like a malformed ProjectionBindingExpression (see stack trace below). I don't think I introduced this bug here, but can you please tell a look and tell me what you think?

Exception stack trace
System.ArgumentOutOfRangeException
Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
   at System.Collections.Generic.List`1.get_Item(Int32 index)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressions.SelectExpression.GetProjection(ProjectionBindingExpression projectionBindingExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs:line 1786
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.VisitExtension(Expression extensionExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 629
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.BindProperty(EntityReferenceExpression entityReferenceExpression, IProperty property) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 1184
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.TryBindMember(Expression source, MemberIdentity member) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 1169
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 747
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 817
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.TryRewriteEntityEquality(ExpressionType nodeType, Expression left, Expression right, Boolean equalsMethod, Expression& result) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 1760
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 820
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.TranslateInternal(Expression expression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 133
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.Translate(Expression expression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 128
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateExpression(Expression expression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 1586
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateLambdaExpression(ShapedQueryExpression shapedQueryExpression, LambdaExpression lambdaExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 1604
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateWhere(ShapedQueryExpression source, LambdaExpression predicate) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 1026
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateAny(ShapedQueryExpression source, LambdaExpression predicate) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 269
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore/Query/QueryableMethodTranslatingExpressionVisitor.cs:line 129
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 215
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.TranslateSubquery(Expression expression) in /home/roji/projects/efcore/src/EFCore/Query/QueryableMethodTranslatingExpressionVisitor.cs:line 511
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 959
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.TranslateInternal(Expression expression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 133
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.Translate(Expression expression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs:line 128
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateExpression(Expression expression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 1586
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateLambdaExpression(ShapedQueryExpression shapedQueryExpression, LambdaExpression lambdaExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 1604
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateWhere(ShapedQueryExpression source, LambdaExpression predicate) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 1026
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore/Query/QueryableMethodTranslatingExpressionVisitor.cs:line 474
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 215
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateExecuteDelete(ShapedQueryExpression source) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 1145
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/efcore/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs:line 200
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query) in /home/roji/projects/efcore/src/EFCore/Query/QueryCompilationContext.cs:line 166
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async) in /home/roji/projects/efcore/src/EFCore/Storage/Database.cs:line 66
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async) in /home/roji/projects/efcore/src/EFCore/Query/Internal/QueryCompiler.cs:line 82
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0() in /home/roji/projects/efcore/src/EFCore/Query/Internal/QueryCompiler.cs:line 66
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler) in /home/roji/projects/efcore/src/EFCore/Query/Internal/CompiledQueryCache.cs:line 66
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query) in /home/roji/projects/efcore/src/EFCore/Query/Internal/QueryCompiler.cs:line 62
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression) in /home/roji/projects/efcore/src/EFCore/Query/Internal/EntityQueryProvider.cs:line 63
   at Microsoft.EntityFrameworkCore.RelationalQueryableExtensions.ExecuteDelete[TSource](IQueryable`1 source) in /home/roji/projects/efcore/src/EFCore.Relational/Extensions/RelationalQueryableExtensions.cs:line 296
   at Microsoft.EntityFrameworkCore.BulkUpdates.NonSharedModelBulkUpdatesTestBase.<>c__DisplayClass20_0`2.<AssertDelete>b__1(TContext context) in /home/roji/projects/efcore/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs:line 246
   at Microsoft.EntityFrameworkCore.TestUtilities.TestHelpers.<>c__DisplayClass34_0`1.<ExecuteWithStrategyInTransaction>b__0(TContext context) in /home/roji/projects/efcore/test/EFCore.Specification.Tests/TestUtilities/TestHelpers.cs:line 297
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.<>c__2`1.<Execute>b__2_0(<>f__AnonymousType0`2 s) in /home/roji/projects/efcore/src/EFCore/Storage/ExecutionStrategyExtensions.cs:line 83
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.<>c__DisplayClass12_0`2.<Execute>b__0(DbContext _, TState s) in /home/roji/projects/efcore/src/EFCore/Storage/ExecutionStrategyExtensions.cs:line 392
   at Microsoft.EntityFrameworkCore.Storage.NonRetryingExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded) in /home/roji/projects/efcore/src/EFCore/Storage/NonRetryingExecutionStrategy.cs:line 75
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, TState state, Func`2 operation, Func`2 verifySucceeded) in /home/roji/projects/efcore/src/EFCore/Storage/ExecutionStrategyExtensions.cs:line 390
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, TState state, Func`2 operation) in /home/roji/projects/efcore/src/EFCore/Storage/ExecutionStrategyExtensions.cs:line 331
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState](IExecutionStrategy strategy, TState state, Action`1 operation) in /home/roji/projects/efcore/src/EFCore/Storage/ExecutionStrategyExtensions.cs:line 77
   at Microsoft.EntityFrameworkCore.TestUtilities.TestHelpers.ExecuteWithStrategyInTransaction[TContext](Func`1 createContext, Action`2 useTransaction, Action`1 testOperation, Action`1 nestedTestOperation1, Action`1 nestedTestOperation2, Action`1 nestedTestOperation3) in /home/roji/projects/efcore/test/EFCore.Specification.Tests/TestUtilities/TestHelpers.cs:line 290
   at Microsoft.EntityFrameworkCore.BulkUpdates.NonSharedModelBulkUpdatesTestBase.AssertDelete[TContext,TResult](Boolean async, Func`1 contextCreator, Func`2 query, Int32 rowsAffectedCount) in /home/roji/projects/efcore/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs:line 240
   at Microsoft.EntityFrameworkCore.BulkUpdates.NonSharedModelBulkUpdatesTestBase.Delete_entity_with_auto_include(Boolean async) in /home/roji/projects/efcore/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs:line 135
   at Microsoft.EntityFrameworkCore.BulkUpdates.NonSharedModelBulkUpdatesSqliteTest.Delete_entity_with_auto_include(Boolean async) in /home/roji/projects/efcore/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs:line 66
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass48_0.<<InvokeTestMethodAsync>b__1>d.MoveNext() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 264
--- End of stack trace from previous location ---

@roji roji dismissed maumar’s stale review March 24, 2023 09:30

Added another commit to fix #30572

@roji roji changed the title Fully unwrap IncludeExpression before ExecuteUpdate Fully unwrap IncludeExpression before ExecuteUpdate/Delete Mar 24, 2023
@maumar
Copy link
Contributor

maumar commented Mar 24, 2023

the new change looks good, I will try to dig into the underlying projection binding issue, but that's orthogonal.

@roji roji changed the title Fully unwrap IncludeExpression before ExecuteUpdate/Delete Fixes around IncludeExpression for ExecuteUpdate/Delete Mar 25, 2023
@roji roji merged commit 4c6f854 into dotnet:main Mar 25, 2023
@roji roji deleted the ExecuteUpdateIncludes branch March 25, 2023 09:00
roji added a commit to roji/efcore that referenced this pull request Mar 25, 2023
* Fully prune IncludeExpression before ExecuteUpdate, not just for the
  property lambda
* Prune also non-owned Includes

Fixes dotnet#30572
Fixes dotnet#30528

(cherry picked from commit 4c6f854)
wtgodbe pushed a commit that referenced this pull request Apr 4, 2023
)

* Fully prune IncludeExpression before ExecuteUpdate, not just for the
  property lambda
* Prune also non-owned Includes

Fixes #30572
Fixes #30528

(cherry picked from commit 4c6f854)
roji added a commit to roji/efcore that referenced this pull request Nov 17, 2023
* Fully prune IncludeExpression before ExecuteUpdate, not just for the
  property lambda
* Prune also non-owned Includes

Fixes dotnet#30572
Fixes dotnet#30528

(cherry picked from commit 4c6f854)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants