Skip to content

Conversation

@cincuranet
Copy link
Contributor

Closes #36357.

@cincuranet cincuranet requested a review from a team as a code owner July 10, 2025 09:21
@cincuranet cincuranet requested review from Copilot and roji July 10, 2025 09:21
Copy link

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.

Pull Request Overview

This PR introduces support for the new EF.MultipleParameters query method by extending the core query pipeline, expression visitors, and SQL generation, and updates tests to exercise the new parameterization modes.

  • Add a ParameterExpressionMode enum and thread it through QueryParameterExpression, normalization, funcletization, and SQL translation.
  • Extend relational and SQL Server nullability and in‐memory processors to handle multiple parameter expansions.
  • Update relational, SQL Server, SQLite, and Cosmos tests to parameterize collection tests over ParameterizedCollectionMode and add new EF.MultipleParameters tests.

Reviewed Changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/EFCore/Query/QueryParameterExpression.cs Replace boolean flags with ParameterExpressionMode property.
src/EFCore/Query/Internal/QueryableMethodNormalizingExpressionVisitor.cs Normalize EF.MultipleParameters calls into QueryParameterExpression with the new mode.
src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs Throw appropriate generic error for non-evaluatable EF.MultipleParameters<T>.
src/EFCore.Relational/Query/SqlNullabilityProcessor.cs Handle multiple parameter expansion based on ParameterExpressionMode.
src/EFCore.Relational/Query/SqlExpressions/SqlParameterExpression.cs Replace ShouldBeConstantized with ParameterExpressionMode.
src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs Apply new mode flags when generating IN and JSON queries.
src/EFCore.Relational/Query/Internal/RelationalParameterProcessor.cs Thread ParameterExpressionMode through parameter processing.
src/EFCore/Properties/CoreStrings.resx Consolidate EF.Constant/EF.Parameter error messages into generic template.
test/.../NonSharedPrimitiveCollections* Convert collection tests to theories over ParameterizedCollectionMode.
test/.../NorthwindWhereQuery* Add EF_MultipleParameters_with_non_evaluatable_argument_throws tests for relational and provider-specific projects.
Files not reviewed (1)
  • src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported

Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

Hey, looks good - see code quailty comments below.

@cincuranet cincuranet requested a review from roji July 15, 2025 10:46
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

See last minor suggestions, otherwise LGTM.

/// Which parametrized collection translation mode should be used.
/// </summary>
public ParameterizedCollectionMode ParameterizedCollectionMode { get; init; }
public ParameterTranslationMode ParameterizedCollectionMode { get; init; }
Copy link
Member

Choose a reason for hiding this comment

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

Given the new naming, maybe call this CollectionParameterTranslationMode for consistency etc. (also elsewhere)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UseParameterizedCollectionMode stays?

Copy link
Member

Choose a reason for hiding this comment

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

I guess in the interest of avoiding another design discussion on the public API at this late hour, let's just keep things as they are - disregard my above comment...

@cincuranet cincuranet merged commit 74068d2 into dotnet:main Jul 16, 2025
7 checks passed
@cincuranet cincuranet deleted the ef-mp branch July 16, 2025 11:21
@cincuranet cincuranet mentioned this pull request Jul 30, 2025
24 tasks
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.

EF.MultipleParameters (only relational) to switch behavior locally in query

2 participants