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.

Sign up for GitHub

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

Jump to bottom

Projection middleware checks for null on non-nullable field, resulting in exception for complex type in EF Core 8 #6604

Open
1 task done
glen-84 opened this issue Oct 14, 2023 · 3 comments · May be fixed by #6706
Open
1 task done

Projection middleware checks for null on non-nullable field, resulting in exception for complex type in EF Core 8 #6604

glen-84 opened this issue Oct 14, 2023 · 3 comments · May be fixed by #6706
Labels
Area: Data Issue is related to filtering, sorting, pagination or projections Area: Projections 🐛 bug Something isn't working 🌶️ hot chocolate

Comments

@glen-84
Copy link
Collaborator

glen-84 commented Oct 14, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Product

Hot Chocolate

Describe the bug

The projection middleware is generating this query:

DbSet<User>()
    .Where(u => u.Id == __query_EntityId_0)
    .Select(_s1 => new User{
        Id = _s1.Id,
        Username = _s1.Username,
        EmailAddress = _s1.EmailAddress,
        AccountStatus = _s1.AccountStatus != null ? new UserAccountStatus{ IsRegistrationCompleted = _s1.AccountStatus.IsRegistrationCompleted }
         : null
    }
    )

Which results in an EF Core 8 exception when using complex types:

System.InvalidOperationException: Comparing complex types to null is not supported.

User.AccountStatus is non-nullable, so it shouldn't be checking for null like this.

Steps to reproduce

I'll provide a reproduction repository if it helps, but it's probably not necessary.

Relevant log output

No response

Additional Context?

No response

Version

13.5.1
@glen-84 glen-84 added the 🐛 bug Something isn't working label Oct 14, 2023
@glen-84
Copy link
Collaborator Author

glen-84 commented Oct 22, 2023

@PascalSenn Would you be able to point to the code that is responsible for adding this null check?

@michaelstaib michaelstaib added 🌶️ hot chocolate Area: Data Issue is related to filtering, sorting, pagination or projections labels Oct 23, 2023
@PascalSenn
Copy link
Member

PascalSenn commented Nov 14, 2023 •
edited

This exception is a bit weird.
If i remember correctly we deliberately added the null-checks back because of some EF Core behaviour.

The visitor is able to "skip all null checks":

graphql-platform/src/HotChocolate/Data/src/Data/Projections/Expressions/Handlers/QueryableProjectionFieldHandler.cs

Lines 93 to 104 in 3f7f160

if (context.InMemory)
{
parentScope.Level
.Peek()
.Enqueue(Expression.Bind(field.Member, NotNullAndAlso(nestedProperty, memberInit)));
}
else
{
parentScope.Level
.Peek()
.Enqueue(Expression.Bind(field.Member, memberInit));
}

but in the case of ef core we added it back:

graphql-platform/src/HotChocolate/Data/src/Data/Projections/Expressions/QueryableProjectionProvider.cs

Line 85 in 3f7f160

// We cannot opt out of the null checks because ef core does not like it

Do you have a stacktrace for the exception?

does it work when we use default?

@glen-84
Copy link
Collaborator Author

glen-84 commented Nov 14, 2023 •
edited 

System.InvalidOperationException: Comparing complex types to null is not supported.
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.<>c__DisplayClass62_0.<TryRewriteStructuralTypeEquality>g__TryRewriteComplexTypeEquality|1(Expression& result)
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.TryRewriteStructuralTypeEquality(ExpressionType nodeType, Expression left, Expression right, Boolean equalsMethod, Expression& result)
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Query.Internal.NpgsqlSqlTranslatingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.VisitConditional(ConditionalExpression conditionalExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.TranslateInternal(Expression expression, Boolean applyDefaultTypeMapping)
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.TranslateProjection(Expression expression, Boolean applyDefaultTypeMapping)
   at Microsoft.EntityFrameworkCore.Query.Internal.RelationalProjectionBindingExpressionVisitor.Visit(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.RelationalProjectionBindingExpressionVisitor.VisitMemberAssignment(MemberAssignment memberAssignment)
   at System.Linq.Expressions.ExpressionVisitor.VisitMemberBinding(MemberBinding node)
   at Microsoft.EntityFrameworkCore.Query.Internal.RelationalProjectionBindingExpressionVisitor.VisitMemberInit(MemberInitExpression memberInitExpression)
   at Microsoft.EntityFrameworkCore.Query.Internal.RelationalProjectionBindingExpressionVisitor.Visit(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.RelationalProjectionBindingExpressionVisitor.Translate(SelectExpression selectExpression, Expression expression)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateSelect(ShapedQueryExpression source, LambdaExpression selector)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.Translate(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.Translate(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass12_0`1.<ExecuteAsync>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.ExecuteAsync[TResult](Expression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetAsyncEnumerator(CancellationToken cancellationToken)
   at HotChocolate.Data.Projections.SingleOrDefaultMiddleware`1.InvokeAsync(IMiddlewareContext context)
   at HotChocolate.Utilities.MiddlewareCompiler`1.ExpressionHelper.AwaitTaskHelper(Task task)
   at HotChocolate.Execution.Processing.Tasks.ResolverTask.ExecuteResolverPipelineAsync(CancellationToken cancellationToken)
   at HotChocolate.Execution.Processing.Tasks.ResolverTask.TryExecuteAsync(CancellationToken cancellationToken)

I believe default has the same issue.

@glen-84 glen-84 linked a pull request Nov 17, 2023 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area: Data Issue is related to filtering, sorting, pagination or projections Area: Projections 🐛 bug Something isn't working 🌶️ hot chocolate
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid checking for null on non-nullable property for projection glen-84/graphql-platform
3 participants
@glen-84 @michaelstaib @PascalSenn