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

Implement EF.Constant #32412

Merged
merged 1 commit into from
Nov 27, 2023
Merged

Implement EF.Constant #32412

merged 1 commit into from
Nov 27, 2023

Conversation

roji
Copy link
Member

@roji roji commented Nov 26, 2023

Closes #31552

@roji roji requested a review from a team November 26, 2023 14:19
@roji roji merged commit 484a3ed into dotnet:main Nov 27, 2023
7 checks passed
@roji roji deleted the EfConstant branch November 27, 2023 20:30
roji added a commit to roji/efcore that referenced this pull request Nov 28, 2023
Closes dotnet#31552

(cherry picked from commit 484a3ed)
roji added a commit to roji/efcore that referenced this pull request Nov 30, 2023
Closes dotnet#31552

(cherry picked from commit 484a3ed)
wtgodbe pushed a commit that referenced this pull request Jan 3, 2024
Closes #31552

(cherry picked from commit 484a3ed)
@clement911
Copy link
Contributor

It's probably a bit late, but I wonder if EF.Inline or EF.Literal would be a more appropriate name, given that it works not only for constant literals but also arrays and collections used in Contains.

@roji
Copy link
Member Author

roji commented Feb 22, 2024

@clement911 it's indeed too late as this has already shipped in 8.0.2; but I'm not seeing how EF.Inline or EF.Literal are better than EF.Constant; "constant" is the name used in the LINQ expression itself (ConstantExpression) - that can contain array or collection values just as well as non-collection ones.

@clement911
Copy link
Contributor

Thanks for your quick reply.

I didn't realize that the Linq expression used the term Constant already... but it's an implementation detail.

I don't know. I find it strange to call a collection a 'constant'.

I also find that 'Inline' would be more effective at conveying the fact that the purpose of the function is to avoid introducing a parameter.

It's no big deal of course.

@roji
Copy link
Member Author

roji commented Feb 23, 2024

I find it strange to call a collection a 'constant'.

Constant here simply implies "not a parameter or column" - it doesn't imply "scalar" or similar, as you seem to be understanding. In LINQ expression trees it's perfectly valid to write e.g. Expression.Constant(new[] { 1, 2, 3 }).

We use inline to convey another distinction: whether the value is constructed in-line within the query; this distinction doesn't make sense for ints/strings, but it does for arrays. To make things clearer:

// This is a parameterized array
var array = new[] { 1, 2, 3 };
_ = ctx.Blogs.Where(b => array.Contains(b.Id).ToList();
// This produces an OPENJSON-based translation in 8.0, since a JSON array is the way to parameterize an array of values.

// This is a constant array:
var array = new[] { 1, 2, 3 };
_ = ctx.Blogs.Where(b => EF.Constant(array).Contains(b.Id).ToList();
// This translates to `b.Id IN (1, 2, 3)`.

// This is an inline array/collection:
_ = ctx.Blogs.Where(b => new[] { 1, 2, b.Id }.Contains(b.Id)).ToList();
// Note that the array very much isn't a constant - it even contains a column as an element. But it's specified in line.

Hopefully that explains the terminology.

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.

Adding EF.Constant to prevent parameterization in query
4 participants