fix #34001: NULL handling in search condition CASE/WHEN

[pumpkin-bit]

fixes #34001 bug issue

• I've read the guidelines for contributing and seen the walkthrough
• I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
• The code builds and tests pass locally (also verified by our automated build checks)
• Commit messages follow this format:

        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber

• Tests for the changes have been added (for bug fixes / features)
• Code follows the same patterns and style as existing code in this repo

Description

The SqlServer SearchConditionConverter previously forced a binary translation CASE WHEN <condition> THEN 1 ELSE 0 END. This caused UNKNOWN evaluations when NULL was involved in the original expression to erroneously fall back to 0 false.

My local test

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

class Program
{
    static void Main()
    {
        using var db = new TestContext();
        var query = db.Entities
            .Select(e => EF.Functions.Like(e.NullableString, "a"));

        Console.WriteLine("Generated SQL for SELECT LIKE:");
        Console.WriteLine("-------");
        Console.WriteLine(query.ToQueryString());
        Console.WriteLine("-------");
        
        var query2 = db.Entities
            .Select(e => e.NullableInt > 0);

        Console.WriteLine("Generated SQL for SELECT > 0:");
        Console.WriteLine("-------");
        Console.WriteLine(query2.ToQueryString());
        Console.WriteLine("-------");
        
        var query3 = db.Entities
            .Select(e => (bool?)(e.NullableInt > 0));

        Console.WriteLine("Generated SQL for SELECT (bool?)(> 0):");
        Console.WriteLine("-------");
        Console.WriteLine(query3.ToQueryString());
        Console.WriteLine("-------");
    }
}

public class TestEntity
{
    public int Id { get; set; }
    public string? NullableString { get; set; }
    public int? NullableInt { get; set; }
}

public class TestContext : DbContext
{
    public DbSet<TestEntity> Entities { get; set; }
    protected override void OnConfiguring(DbContextOptionsBuilder options)
    {
        options.UseSqlServer("Server=unreachable;Database=dummy;Integrated Security=True;", o => o.UseRelationalNulls(true));
    }
    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<TestEntity>().HasNoKey();
    }
}

---

Log test

Generated SQL for SELECT LIKE:
-------
SELECT CASE
    WHEN [e].[NullableString] LIKE N'a' THEN CAST(1 AS bit)
    WHEN [e].[NullableString] NOT LIKE N'a' THEN CAST(0 AS bit)
END
FROM [Entities] AS [e]
-------
Generated SQL for SELECT > 0:
-------
SELECT CASE
    WHEN [e].[NullableInt] > 0 THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
FROM [Entities] AS [e]
-------
Generated SQL for SELECT (bool?)(> 0):
-------
SELECT CASE
    WHEN [e].[NullableInt] > 0 THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
FROM [Entities] AS [e]
-------

[roji]

/cc @ranma42

[ranma42]

IIRC this approach works, but it is very likely to cause significant regressions, because it duplicates the subexpression and always introduces the NULL case even when not needed (non-nullable sub-expressions).
Updating the baselines would likely make it more obvious.

[pumpkin-bit]

IIRC this approach works, but it is very likely to cause significant regressions, because it duplicates the subexpression and always introduces the NULL case even when not needed (non-nullable sub-expressions). Updating the baselines would likely make it more obvious.

Yes, I completely agree with you to fix this, you could use a three state case wrapper only for expressions that can actually be null. The problem is that SearchConditionConverter runs very late in the tree generation process. The SqlExpression object doesn't have a property like IsNullable all null analysis happens much earlier in the SqlNullabilityProcessor class. Therefore, I can't simply write an if (sqlExpression.IsNullable) and etc. condition to check.
My question is, what's the best approach to solve this situation? Is it possible to somehow pass nullability information to SearchConditionConverter?

[roji]

Is it possible to somehow pass nullability information to SearchConditionConverter?

For this, see #33889 - this is something I believe we should definitely consider. But it's a really non-trivial architectural change to the way the query pipeline works.

[pumpkin-bit]

Thanks for the clarification and for pointing out #33889.
Since a proper solution to #34001 without duplicating AST queries requires some non-trivial architectural changes, I'll close this PR for now to keep the project board clean. I look forward to the development of the new query pipeline.