Skip to content

Change ExecuteUpdate to accept non-expression lambda#35257

Merged
roji merged 1 commit intodotnet:mainfrom
roji:ExecuteUpdateLambda
Jan 11, 2025
Merged

Change ExecuteUpdate to accept non-expression lambda#35257
roji merged 1 commit intodotnet:mainfrom
roji:ExecuteUpdateLambda

Conversation

@roji
Copy link
Copy Markdown
Member

@roji roji commented Dec 2, 2024

This PR changes ExecuteUpdate to accept Func<SetPropertyCalls<TSource>, SetPropertyCalls<TSource>> instead of Expression<Func<SetPropertyCalls<TSource>, SetPropertyCalls<TSource>>>. This allows mainly for easy dynamic composition of setters (but has other advantages), and is more in-line with how LINQ operators generally work.

  • ExecuteUpdate needs to add an expression node to the tree just like any other operator, so that that node can get translated properly. Previously we just got an expression tree directly as the argument, but now get a Func; so this Func must be evaluated immediately, and its results (which are a set of property/value selector pairs) integrated into the tree.
  • The funcletizer must also be able to properly process the tree fragment produced by ExecuteUpdate; this means that we can't e.g. simply shove a ConstantExpression into it. Instead, we construct a NewArrayExpression representing all the tuples (the funcletizer will visit into it), and later in translation we "parse" that back into the property/value selectors, to be passed to TranslateExecuteUpdate.
  • Precompilation of this new mode is tricky in various ways...
    • Before, ExecuteUpdate just got a single expression tree argument, so it was the same as all other LINQ operators; but we now have a non-expression argument, which, when evaluated, produces an expression argument. This requires special handling in precompilation, to parse the user's SetProperty() calls and actually evaluate them, just as the ExecuteUpdate operator implementation does for non-precompilation.
    • In effect, precompilation needs to do for ExecuteUpdate exactly what we have been doing for non-precompilation - the code just moves around.

Closes #32018
Closes #35361

@roji roji force-pushed the ExecuteUpdateLambda branch from 879c7e5 to f378a22 Compare December 5, 2024 12:20
@roji roji marked this pull request as ready for review December 5, 2024 12:20
@roji roji requested a review from a team as a code owner December 5, 2024 12:20
@roji
Copy link
Copy Markdown
Member Author

roji commented Dec 5, 2024

@ajcvickers this should be ready if you want to take a look.

@roji
Copy link
Copy Markdown
Member Author

roji commented Dec 20, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

No pipelines are associated with this pull request.

@roji roji closed this Dec 20, 2024
@roji roji reopened this Dec 20, 2024
@roji
Copy link
Copy Markdown
Member Author

roji commented Dec 20, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

No pipelines are associated with this pull request.

@roji
Copy link
Copy Markdown
Member Author

roji commented Dec 21, 2024

@AndriySvyryd any idea why this PR refuses to build (I'm seeing this happening with other open PRs at the moment)?

@AndriySvyryd
Copy link
Copy Markdown
Member

@AndriySvyryd any idea why this PR refuses to build (I'm seeing this happening with other open PRs at the moment)?

Probably an AZDO outage

@roji
Copy link
Copy Markdown
Member Author

roji commented Dec 29, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@AndriySvyryd AndriySvyryd requested a review from Copilot January 4, 2025 01:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 23 out of 38 changed files in this pull request and generated no comments.

Files not reviewed (15)
  • src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported
  • src/EFCore/Properties/CoreStrings.resx: Language not supported
  • src/EFCore.Design/Query/Internal/CSharpToLinqTranslator.cs: Evaluated as low risk
  • src/EFCore.Design/Query/Internal/PrecompiledQueryCodeGenerator.cs: Evaluated as low risk
  • src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.ExecuteUpdate.cs: Evaluated as low risk
  • src/EFCore.Relational/Query/SqlNullabilityProcessor.cs: Evaluated as low risk
  • src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs: Evaluated as low risk
  • src/EFCore/Query/QueryableMethodTranslatingExpressionVisitor.cs: Evaluated as low risk
  • test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs: Evaluated as low risk
  • test/EFCore.SqlServer.FunctionalTests/BulkUpdates/ComplexTypeBulkUpdatesSqlServerTest.cs: Evaluated as low risk
  • test/EFCore.Specification.Tests/TestUtilities/BulkUpdatesAsserter.cs: Evaluated as low risk
  • test/EFCore.Specification.Tests/BulkUpdates/NorthwindBulkUpdatesTestBase.cs: Evaluated as low risk
  • test/EFCore.Specification.Tests/BulkUpdates/BulkUpdatesTestBase.cs: Evaluated as low risk
  • test/EFCore.Relational.Specification.Tests/TestUtilities/BulkUpdatesAsserter.cs: Evaluated as low risk
  • test/EFCore.Relational.Specification.Tests/BulkUpdates/NorthwindBulkUpdatesRelationalTestBase.cs: Evaluated as low risk

Comment thread src/EFCore/Query/SetPropertyCalls.cs Outdated
@roji roji force-pushed the ExecuteUpdateLambda branch from b882a4e to aeeb86d Compare January 11, 2025 09:30
@roji roji enabled auto-merge (squash) January 11, 2025 09:31
@roji roji merged commit d96dde2 into dotnet:main Jan 11, 2025
@roji roji deleted the ExecuteUpdateLambda branch January 11, 2025 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The order of ExecuteUpdate setters is not preserved Accept non-expression setters parameter in ExecuteUpdate

3 participants