dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.64k stars 3.15k forks source link

System.NotImplementedException with precompiledquery tests #34393

Open ChrisJollyAU opened 1 month ago

ChrisJollyAU commented 1 month ago

I am busy adding the PrecompiledQuery functionality to EFCore.Jet provider including its test suite and have run into an error I'm not sure about

When running a test (e.g. BinaryExpression) in PrecompiledQueryJetTest (outside of naming and such pretty much similar to PrecompiledQuerySqlServerTest), I end up with the following error

 Precompilation error: System.NotImplementedException: The method or operation is not implemented.
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitTry(TryExpression tryNode)
   at System.Linq.Expressions.TryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Translate[T](Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateList(IReadOnlyList`1 list)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitNewArray(NewArrayExpression newArray)
   at System.Linq.Expressions.NewArrayExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
...... stack trace with a bunch of Visits
at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateCore(Expression node, IReadOnlyDictionary`2 constantReplacements, ISet`1 collectedNamespaces, ISet`1 unsafeAccessors, Boolean statementContext)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateExpression(Expression node, IReadOnlyDictionary`2 constantReplacements, ISet`1 collectedNamespaces, ISet`1 unsafeAccessors)
   at Microsoft.EntityFrameworkCore.Query.Internal.RuntimeModelLinqToCSharpSyntaxTranslator.TranslateExpression(Expression node, IReadOnlyDictionary`2 constantReplacements, IReadOnlyDictionary`2 memberAccessReplacements, ISet`1 collectedNamespaces, ISet`1 unsafeAccessors)
   at Microsoft.EntityFrameworkCore.Query.Internal.PrecompiledQueryCodeGenerator.GenerateQueryExecutor(IndentedStringBuilder code, Int32 queryNum, Expression queryExecutor, HashSet`1 namespaces, HashSet`1 unsafeAccessors)
   at Microsoft.EntityFrameworkCore.Query.Internal.PrecompiledQueryCodeGenerator.ProcessSyntaxTree(SyntaxTree syntaxTree, SemanticModel semanticModel, IReadOnlyList`1 locatedQueries, List`1 precompilationErrors, String suffix, ISet`1 generatedFileNames, CancellationToken cancellationToken)

  Stack Trace: 
PrecompiledQueryTestHelpers.FullSourceTest(String sourceCode, DbContextOptions dbContextOptions, Type dbContextType, Action`1 interceptorCodeAsserter, Action`1 errorAsserter, ITestOutputHelper testOutputHelper, Boolean alwaysPrintGeneratedSources, String callerName)
PrecompiledQueryTestHelpers.FullSourceTest(String sourceCode, DbContextOptions dbContextOptions, Type dbContextType, Action`1 interceptorCodeAsserter, Action`1 errorAsserter, ITestOutputHelper testOutputHelper, Boolean alwaysPrintGeneratedSources, String callerName)
PrecompiledQueryJetTest.BinaryExpression() line 31
--- End of stack trace from previous location ---

It obviously doesn't happen with Sql Server or Sqlite.

Debugging info This seems to have something to do with the Execution Strategy being used. When it uses the default NonRetryingExecutionStrategy it comes up with the above error. However if I use the retrying execution strategy (JetRetryingExecutionStrategy or TestJetRetryingExecutionStrategy) it works fine as expected.

Further debugging brings me to the CompileQuery function within the ProcessSyntaxTree in this file https://github.com/dotnet/efcore/blob/main/src/EFCore.Design/Query/Internal/PrecompiledQueryCodeGenerator.cs#L188

It is further in the ProcessSyntaxTree function when it calls GenerateQueryExecutor that we get the error.

Looking at the debug view of the queryExecutor variable we see the following difference (only relevant part shown - not the whole view) This is what I get with Sql Server and Sqlite

$materializationContext1 = .New Microsoft.EntityFrameworkCore.Storage.MaterializationContext(
                $emptyValueBuffer,
                $queryContext.Context);
            $instance1 = .Default(Microsoft.EntityFrameworkCore.Query.PrecompiledQueryRelationalTestBase+Blog);
            $entry1 = .Call $queryContext.TryGetEntry(
                $key,
                .NewArray System.Object[] {
                    (System.Object).Call $dataReader.GetInt32(0)
                },
                True,
                $hasNullKey1);
            .If (
                !$hasNullKey1

This is what I get on EFCore.Jet

$materializationContext1 = .New Microsoft.EntityFrameworkCore.Storage.MaterializationContext(
                $emptyValueBuffer,
                $queryContext.Context);
            $instance1 = .Default(Microsoft.EntityFrameworkCore.Query.PrecompiledQueryRelationalTestBase+Blog);
            $entry1 = .Call $queryContext.TryGetEntry(
                $key,
                .NewArray System.Object[] {
                    .Try {
                        (System.Object).Call $dataReader.GetInt32(0)
                    } .Catch (System.Exception $e) {
                        .Call Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor+ShaperProcessingExpressionVisitor.ThrowReadValueException(
                            $e,
                            .Call $dataReader.GetFieldValue(0),
                            .Constant<System.Type>(System.Object),
                            $'property: Blog.Id (int) Required PK AfterSave:ThrowProperty')
                    }
                },
                True,
                $hasNullKey1);
            .If (
                !$hasNullKey1

Note the part where the data reader call is wrapped in a try...catch. Not sure why it is being generated

As a further note, in the VisitTry function of LinqToCSharpSyntaxTranslator, the _context variable that the switch is done on is ExpressionContext.Expression hence the NotImplementedException

I know its still marked as experimental so maybe I have hit something not finished yet

SDK: .Net 9 preview 6 Visual Studio: 17.11.0 preview 7 EF Core: 9.0.0-rc.1.24374.2 ( a daily build a day or 2 after the last preview 7 so should be close to what the final preview 7 release will be) Using the preview 6 branch of efcore to build and debug on the SQL Server and Sqlite for comparison

Any thoughts?

ChrisJollyAU commented 1 month ago

Bit more info on this

Its actually part of the shaper created in RelationalShapedQueryCompilingExpressionVisitor (under EFCore.Relational\Query)

It is part of the function CreateGetValueExpression

When you have detailed erros enabled you have the following block that executes of which it doesn't seem like the precompile undewrstands it yet

if (_detailedErrorsEnabled
    && !buffering)
{
    var exceptionParameter = Parameter(typeof(Exception), name: "e");

    var catchBlock = Catch(
        exceptionParameter,
        Call(
            ThrowReadValueExceptionMethod.MakeGenericMethod(valueExpression.Type),
            exceptionParameter,
            Call(dbDataReader, GetFieldValueMethod.MakeGenericMethod(typeof(object)), indexExpression),
            Constant(valueExpression.Type.MakeNullable(nullable), typeof(Type)),
            _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
                property,
                LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForProperty(property),
                property + "Property",
                typeof(IPropertyBase))));

    valueExpression = TryCatch(valueExpression, catchBlock);
}

It is simple to recreate this issue - just add .EnableDetailedErrors() to the builder in AddProviderOptions. e.g. with Sqlite you will have

public virtual DbContextOptionsBuilder AddProviderOptions(
    DbContextOptionsBuilder builder,
    Action<SqliteDbContextOptionsBuilder>? configureSqlite)
    => builder.UseSqlite(
        Connection, b =>
        {
            b.CommandTimeout(CommandTimeout);
            b.UseQuerySplittingBehavior(QuerySplittingBehavior.SingleQuery);
            configureSqlite?.Invoke(b);
        }).EnableDetailedErrors();