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

Exception when SELECTing (x ? A : default(int?) >= B) #33752

Closed
ranma42 opened this issue May 18, 2024 · 4 comments · Fixed by #33757
Closed

Exception when SELECTing (x ? A : default(int?) >= B) #33752

ranma42 opened this issue May 18, 2024 · 4 comments · Fixed by #33757
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@ranma42
Copy link
Contributor

ranma42 commented May 18, 2024

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
Copy link
Contributor

pmiddleton commented May 18, 2024

@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
Copy link
Contributor Author

ranma42 commented May 19, 2024

@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!

ranma42 added a commit to ranma42/efcore that referenced this issue May 19, 2024
As reported in dotnet#33752, `SELECT`ing the result of a comparison between `int?` and
`int` can trigger an exception in the shaper.
ranma42 added a commit to ranma42/efcore that referenced this issue May 19, 2024
In C# an ordered comparison (<, >, <=, >=) between two nullable values always
returns a boolean value: if either operand is null, the result is false;
otherwise, the result is that of the comparison of the (non-null) values.

Fixes dotnet#33752
ranma42 added a commit to ranma42/efcore that referenced this issue May 20, 2024
As reported in dotnet#33752, `SELECT`ing the result of a comparison between `int?` and
`int` can trigger an exception in the shaper.
ranma42 added a commit to ranma42/efcore that referenced this issue May 20, 2024
In C# an ordered comparison (<, >, <=, >=) between two nullable values always
returns a boolean value: if either operand is null, the result is false;
otherwise, the result is that of the comparison of the (non-null) values.

Fixes dotnet#33752
@roji roji unassigned roji and cincuranet May 20, 2024
@roji
Copy link
Member

roji commented May 20, 2024

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

@maumar
Copy link
Contributor

maumar commented May 20, 2024

related to #31020

ranma42 added a commit to ranma42/efcore that referenced this issue May 22, 2024
As reported in dotnet#33752, `SELECT`ing the result of a comparison between `int?` and
`int` can trigger an exception in the shaper.
ranma42 added a commit to ranma42/efcore that referenced this issue May 22, 2024
In C# an ordered comparison (<, >, <=, >=) between two nullable values always
returns a boolean value: if either operand is null, the result is false;
otherwise, the result is that of the comparison of the (non-null) values.

Fixes dotnet#33752
ranma42 added a commit to ranma42/efcore that referenced this issue May 24, 2024
As reported in dotnet#33752, `SELECT`ing the result of a comparison between `int?` and
`int` can trigger an exception in the shaper.
ranma42 added a commit to ranma42/efcore that referenced this issue May 24, 2024
In C# an ordered comparison (<, >, <=, >=) between two nullable values always
returns a boolean value: if either operand is null, the result is false;
otherwise, the result is that of the comparison of the (non-null) values.

Fixes dotnet#33752
ranma42 added a commit to ranma42/efcore that referenced this issue May 25, 2024
As reported in dotnet#33752, `SELECT`ing the result of a comparison between `int?` and
`int` can trigger an exception in the shaper.
ranma42 added a commit to ranma42/efcore that referenced this issue May 25, 2024
In C# an ordered comparison (<, >, <=, >=) between two nullable values always
returns a boolean value: if either operand is null, the result is false;
otherwise, the result is that of the comparison of the (non-null) values.

Fixes dotnet#33752
ranma42 added a commit to ranma42/efcore that referenced this issue May 29, 2024
As reported in dotnet#33752, `SELECT`ing the result of a comparison between `int?` and
`int` can trigger an exception in the shaper.
ranma42 added a commit to ranma42/efcore that referenced this issue May 29, 2024
In C# an ordered comparison (<, >, <=, >=) between two nullable values always
returns a boolean value: if either operand is null, the result is false;
otherwise, the result is that of the comparison of the (non-null) values.

Fixes dotnet#33752
ranma42 added a commit to ranma42/efcore that referenced this issue May 29, 2024
As reported in dotnet#33752, `SELECT`ing the result of a comparison between `int?` and
`int` can trigger an exception in the shaper.
ranma42 added a commit to ranma42/efcore that referenced this issue May 29, 2024
In C# an ordered comparison (<, >, <=, >=) between two nullable values always
returns a boolean value: if either operand is null, the result is false;
otherwise, the result is that of the comparison of the (non-null) values.

Fixes dotnet#33752
ranma42 added a commit to ranma42/efcore that referenced this issue May 30, 2024
As reported in dotnet#33752, `SELECT`ing the result of a comparison between `int?` and
`int` can trigger an exception in the shaper.
ranma42 added a commit to ranma42/efcore that referenced this issue May 30, 2024
In C# an ordered comparison (<, >, <=, >=) between two nullable values always
returns a boolean value: if either operand is null, the result is false;
otherwise, the result is that of the comparison of the (non-null) values.

Fixes dotnet#33752
@maumar maumar closed this as completed in b77d2f4 May 31, 2024
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 31, 2024
@maumar maumar added this to the 9.0.0 milestone May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants