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

Precompiled query inner loop: source generator to warn and fail dynamic queries #32727

Open
roji opened this issue Jan 5, 2024 · 1 comment · May be fixed by #33350
Open

Precompiled query inner loop: source generator to warn and fail dynamic queries #32727

roji opened this issue Jan 5, 2024 · 1 comment · May be fixed by #33350

Comments

@roji
Copy link
Member

roji commented Jan 5, 2024

Our implementation of precompiled queries will generate LINQ operator interceptors at publishing time, where we can have access to the EF model and perform long query compilation tasks (both of which aren't possible from a Roslyn source generator). We will only be supporting "static" queries:

// OK query:
_ = context.Blogs.Where(...).OrderBy(...).ToList();

// unsupported query query:
var query = context.Blogs.Where(...).OrderBy(...);
_ = query.ToList(); 

However, we must also make sure that the user's program works in the same way in the dev inner loop as it does in the final published version. In other words, if the user's code has a dynamic LINQ query which cannot be precompiled, that query needs to fail (and ideally also be warned about) in the inner loop as well.

To do this, we'll introduce an (incremental) source generator which detects EF query terminating operators, and generates interceptors for them which add an additional "safe marker" expression node on top; this marker indicates that the query is fully static and can be handled at publishing time. Then, in the query pipeline, when NativeAOT is enabled (or some other feature switch which says "precompiled only"), we'll throw for any query which does not have this marker node. This ensures that all queries fail by default, unless they're specifically identified as being static according to our definition. The same source generator can also issue warnings for unsafe queries, for compile-time detection (but note that this works only for the user's own code, and not for any queries executed from dependencies).

Previous write-up

Once we implement precompiled queries, we can introduce a new analyzer to flag queries which aren't compatible with that mechanism (any query which contains any form of dynamicity). Concretely, this means that the query comprises of a single LINQ operation invocation chain, from a root DbSet to a terminating operator. Any terminating operator accepting an IQueryable that doesn't doesn't ultimately hang off of a DbSet is a broken-up, potentially dynamic query:

// OK query:
_ = context.Blogs.Where(...).OrderBy(...).ToList();

// Flagged query:
var query = context.Blogs.Where(...).OrderBy(...);
// Method with an IEnumerable parameter accepting an IQueryable argument, which does not trace back to an inline DbSet root
_ = query.ToList(); 

Note that the analyzer provides a non-exhaustive 1st pass; the actual query precompilation performed at publish time may discover more issues that the answer did not. For example, if some custom/unknown LINQ operator is used somewhere in the query, the analyzer wouldn't be able to flag that (it doesn't compile the query), but query precompilation will fail, raising a warning. This is a bit similar to how the trimming/NativeAOT analyzer(s) may not produce all warnings that the linker does, e.g. because they don't have visibility into the entire code being linked, only to the current project.

Finally, neither the analyzer nor precompilation can provide a 100% guarantee that the program will run correctly with NativeAOT. Doing so isn't possible: the user's program may have a dependency on a 3rd-party package which uses LINQ queries, and there's no way for us to analyze or flag those. More generally, LINQ is an abstraction, and since queryable/EF LINQ queries invoke the same methods (e.g. ToList()) as regular LINQ-to-Objects, the usual annotation-based approach doesn't work here.

Note: the analyzer can also propose a code fix for foreach directly on DbSet properties, which cannot be intercepted as there's no method to intercept:

foreach (var blog in context.Blogs)

The codefix would be to add AsEnumerable().

@ajcvickers ajcvickers added this to the 9.0.0 milestone Jan 17, 2024
@roji roji changed the title Analyzer to warn about dynamic queries (incompatible with precompilation) Analyzer/source gen to warn about dynamic queries (incompatible with precompilation) Mar 14, 2024
@roji roji changed the title Analyzer/source gen to warn about dynamic queries (incompatible with precompilation) Analyzer/source gen to warn and fail dynamic queries (incompatible with precompilation) Mar 17, 2024
@roji roji changed the title Analyzer/source gen to warn and fail dynamic queries (incompatible with precompilation) Precompiled query inner loop: source generator to warn and fail dynamic queries Mar 17, 2024
@roji
Copy link
Member Author

roji commented Mar 17, 2024

Note: updated write-up above to match our plan.

roji added a commit to roji/efcore that referenced this issue Mar 18, 2024
@roji roji linked a pull request Mar 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants