Repository navigation
Fix precompiled queries that are the receiver of an instance method call - #39331
Open
nurullahturkoglu wants to merge 1 commit into
Open
nurullahturkoglu wants to merge 1 commit into
nurullahturkoglu wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #37961
I posted the plan in #37961 (comment) but haven't heard back from the team yet, so the second box is unchecked.
Description
CSharpToLinqTranslator.Translategets the captured variables fromAnalyzeDataFlow(node).DataFlowsIn. When the query is the receiver of an instance method call, such as.ConfigureAwait(false)or.ToString(), Roslyn returnsSucceeded == falsewith an emptyDataFlowsIn, and the translator then fails with "unknown identifier". It's a Roslyn regression from dotnet/roslyn#67913 (4.5 works, 4.7 doesn't), reported in dotnet/roslyn#86099 with a fix in dotnet/roslyn#86100.When the analysis fails, the translator now retries it on the enclosing expressions until one succeeds. A wider region can bring in variables the query doesn't use, but
_dataFlowsInis only looked up for symbols the query references, so those entries are ignored. Once the Roslyn fix is picked up, the retry is no longer needed for this case.The synchronous case, e.g.
context.Blogs.Count(b => b.Id == id).ToString(), fails the same way, so it's covered as well.Testing
I added
Terminating_with_ConfigureAwait(the case from the issue) andTerminating_followed_by_instance_method_call(synchronous) toPrecompiledQueryRelationalTestBase, with SQL Server overrides.Without the fix, both tests fail on SQLite ("unknown identifier 'token'" and "'id'") and on SQL Server. With the fix, the SQLite and SQL Server precompiled query tests pass (SQL Server ran against a local Linux container on macOS), as does
EFCore.Design.Tests.I haven't run the full functional test suites, and I haven't run anything on Windows.