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

Use interceptors to redirect the query to precompiled code #31331

Closed
8 tasks
AndriySvyryd opened this issue Jul 22, 2023 · 2 comments · Fixed by #33297
Closed
8 tasks

Use interceptors to redirect the query to precompiled code #31331

AndriySvyryd opened this issue Jul 22, 2023 · 2 comments · Fixed by #33297
Assignees
Labels
area-aot closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement

Comments

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Jul 22, 2023

See https://github.com/dotnet/roslyn/blob/main/docs/features/interceptors.md

  • Identify and properly intercept all terminating operators
    • ExecuteUpdate/Delete
  • Use ISyntaxGenerator in LinqToCSharpSyntaxGenerator, to avoid e.g. parentheses/operator precedence issues.

Specific query types to keep in mind and test:

  • SQL queries
  • Split query
  • GroupBy final operator (materializer work)
  • Shared-type entity types (in various places where we do entity lookups)
  • Improve tests to assert on results, possibly on SQL.
@roji
Copy link
Member

roji commented Jul 22, 2023

We had a phase where we discussed this... Back then I thought it wouldn't be possible, but I'm not sure any more.

The main problem here is that we need to extract parameters from lambdas in the various LINQ operators that make up the query:

x = context.Blogs.Where(b => b.Name = foo).ToList();

So Where would have to be replaced with something that actually goes and extracts the parameter value, populating it into a QueryContext that flows from the root. It's certainly not trivial stuff, but may be doable - and it would allow us to get compiled query perf (or very near it) with regular queries in code.

Definitely something we should think about.

@ajcvickers ajcvickers added this to the Backlog milestone Aug 2, 2023
@roji roji removed this from the Backlog milestone Jan 5, 2024
@roji roji self-assigned this Jan 5, 2024
@roji
Copy link
Member

roji commented Jan 5, 2024

Note: there's one specific query type which interceptors won't support - foreach over a DbSet property:

foreach (var blog in context.Blogs)
{
    ...
}

The workaround here would be for the user to add AsEnumerable(), which we'd then intercept. A precompilation analyzer could provide a codefix (#32727).

@ajcvickers ajcvickers added this to the 9.0.0 milestone Jan 10, 2024
roji added a commit to roji/efcore that referenced this issue Mar 12, 2024
roji added a commit to roji/efcore that referenced this issue Apr 7, 2024
roji added a commit to roji/efcore that referenced this issue Apr 7, 2024
roji added a commit to roji/efcore that referenced this issue Apr 8, 2024
roji added a commit to roji/efcore that referenced this issue Apr 8, 2024
roji added a commit to roji/efcore that referenced this issue Apr 8, 2024
roji added a commit to roji/efcore that referenced this issue Apr 8, 2024
roji added a commit to roji/efcore that referenced this issue Apr 8, 2024
roji added a commit to roji/efcore that referenced this issue Apr 13, 2024
roji added a commit to roji/efcore that referenced this issue Apr 29, 2024
roji added a commit that referenced this issue Apr 29, 2024
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview5 May 2, 2024
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-aot closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants