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.41k stars 3.11k forks source link

Exception when `SELECT`ing `(x ? A : default(int?) >= B)` #33752

Closed ranma42 closed 2 days ago

ranma42 commented 2 weeks ago

Comparing the result of a ternary operator can result in inconsistent nullability and exceptional termination while scanning the results of a query. Note that filtering by the same Expr does not trigger the issue.

An example program that showcases the bug (and can be conveniently run on https://dotnetfiddle.net/ ;) ) is:

using System;
using System.Data;
using System.Linq;
using Microsoft.EntityFrameworkCore;

using var db = new BloggingContext();

db.Database.EnsureDeleted();
db.Database.EnsureCreated();

db.Blogs
    .Select(x => (x.Url == "http://efcore/" ? 3 : default(int?)) >= 3)
    .ToList();

public class BloggingContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder options)
        => options
            .LogTo(Console.WriteLine, Microsoft.Extensions.Logging.LogLevel.Information)
            .UseSqlite($"Data Source=test.db");

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Blog>().HasData(new Blog { BlogId = 1, Url = "http://efcore/" });
        modelBuilder.Entity<Blog>().HasData(new Blog { BlogId = 2, Url = "trigger ELSE -> NULL" });
    }
}

public class Blog
{
    public int BlogId { get; set; }
    public required string Url { get; set; }
}

Exception

The program terminates with the following exception:

fail: 05/18/2024 11:27:22.780 CoreEventId.QueryIterationFailed[10100] (Microsoft.EntityFrameworkCore.Query) 
      An exception occurred while iterating over the results of a query for context type 'BloggingContext'.
      System.InvalidOperationException: Nullable object must have a value.
         at lambda_method19(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
         at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
Unhandled exception. System.InvalidOperationException: Nullable object must have a value.
   at lambda_method19(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Program.<Main>$(String[] args)

Include provider and version information

EF Core version: 8.0.5 Database provider: Microsoft.EntityFrameworkCore.Sqlite Target framework: .NET 8.0 Operating system: Linux (/WSL) IDE: Visual Studio Code 1.89.1

pmiddleton commented 2 weeks ago

@roji

I just happen to be in this section of the codebase today as I am working through a similar issue I am having in the Windowing functions support. I decided to take a quick peek.

The cause of the exception is that the generated sql allows a null to bleed out which then causes a type mismatch. The shaper lambda then throws an exception due to a bad cast of null => boolean

Sqlite is generating the following which allows a null return value.

SELECT CASE
    WHEN "e"."Name" = 'paul' THEN 3
    ELSE NULL
END >= 3 AS "Id"
FROM "Employees" AS "e"

On Sql Server this is not valid syntax due to the >= 3. The Sql Server code path wraps everything in another case to get this valid syntax.

SELECT CASE
    WHEN CASE
        WHEN [e].[Name] = N'foo' THEN 3
        ELSE NULL
    END >= 3 THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END AS [Id]
FROM [Employees] AS [e]

This syntax is also valid on Sqlite and stops the null from escaping. Applying this same manipulation is probably the correct fix - that exercise is left to the reader :)

Sql Server handels this case by overriding the Optimize function in SqlServerParameterBasedSqlProcessor. SqlServerParameterBasedSqlProcessor is then using the SearchConditionConvertingExpressionVisitor which applies the extra case statement via the VisitSqlBinary -> ApplyConversion -> ConvertToValue call chain.

I hope this helps

Paul

ranma42 commented 2 weeks ago

@pmiddleton thank you! I tried and moved the SearchConditionConvertingExpressionVisitor.Visit() call to RelationalParameterBasedSqlProcessor.Optimize() and indeed the exception is gone!

I will investigate it a little further, because this change (at least in the naive way I applied it) also seems to worsen other queries (introducing stuff like = 1), but I believe you put me on the right track!

roji commented 1 week ago

Thanks for the input @pmiddleton, makes sense! @maumar assigning to you as this is in your traditional area of stewardship :)

maumar commented 1 week ago

related to https://github.com/dotnet/efcore/issues/31020