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

Query: Use parameterization when Contains() only has a small number of elements #6809

Closed
rmja opened this issue Oct 18, 2016 · 7 comments
Closed

Comments

@rmja
Copy link

rmja commented Oct 18, 2016

When filtering using Contains() on a collection with only a single item, the generated SQL should treat it as parameterized SQL to minimize the number of execution plans.

For example:

var ids = new[] {1,2};
await Context.Cars.Where(x => ids.Contains(x.Id)).ToListAsync();

Correctly produces SQL with WHERE Id IN (1,2). However, if the ids array only had one item, say ids = new[] {1}, it would be nice if the generated SQL did not use WHERE IN (1), but instead equality on the Id, using a parameterized version. Something like WHERE Id = @p0.

EF Core version: 1.0.0

@popcatalin81
Copy link

@rmja for Sql Server 2005 + it does not matter, Queries where the discriminator is the clustered key are auto parametrized to use clustered index seek plan. If you analyze the execution plans for your examples you will see the same execution plan for all cases.

@rowanmiller
Copy link
Contributor

@rmja did you have a specific scenario where this would provide a meaningful performance improvement? We're willing to consider it if there is a tangible benefit.

@rmja
Copy link
Author

rmja commented Oct 19, 2016

@rowanmiller In my scenario I usually query by a single id, but also by a few groups of id's. The handling logic in my app is the same, which is why I would be interested in the improvement.

I am seeing a similar behaviour as in this, where my query cache gets polluted by extremely many queries where only the id varies. If I instead use SingleOrDefault() with a specific id I get a much more clean query cache.

I could of cause do the check in the app and branch depending on the count, but it would be more convenient if ef handled this internally.

@divega
Copy link
Contributor

divega commented Oct 20, 2016

As I think @ajcvickers mentioned today in triage we could consider having a threshold (likely a small number greater than 1) under which we could create parameterized queries for the translation of Enumerable.Contains().

Even assuming @popcatalin81's comment is correct this should still help when the query is not auto-parameterized (e.g. if is not on a clustered index), with other providers besides SQL Server and hopefully to avoid regenerating the SQL inside our own stack (see #4605 for some context).

This sounds like a potentially lower impact approach that would still yield a benefit in many common scenarios compared to other options we have considered in the past such as using TVPs.

@rmja
Copy link
Author

rmja commented Oct 20, 2016

Yes, having a per-context configurable threshold sounds like a good approach.

@rowanmiller rowanmiller added this to the Backlog milestone Oct 24, 2016
@rowanmiller rowanmiller changed the title Perf: Use parameterized filter when collection in Contains() only has one element Query: Use parameterization when Contains() only has a small number of elements Oct 24, 2016
@divega
Copy link
Contributor

divega commented Sep 21, 2018

@smitpatel do you think this can be closed now as a duplicate of #12777 that you filed? It seems the latter covers more than the case in which the number of items is relatively small.

@smitpatel
Copy link
Contributor

Duplicate of #12777

@smitpatel smitpatel marked this as a duplicate of #12777 Sep 21, 2018
@smitpatel smitpatel removed this from the Backlog milestone Sep 21, 2018
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants