Value conversion from null in the store generates bad queries

[ajcvickers]

Similar to #26209, but in this case conversion is from non-null in the database to null in code.

Note that this could be another reason not to convert nulls. I don't expect us to do anything about this in 6.0. Maybe ever.

Value converter replaces zeros in the database with null in the model:

    public class ZeroToNullConverter : ValueConverter<int?, int>
    {
        public ZeroToNullConverter()
            : base(
                v => v ?? 0,
                v => v == 0 ? null : v,
                convertsNulls: true)
        {
        }
    }

Query:

var cars = context.Cars.Where(e => e.OwnerId == null).ToList();

Generates SQL:

      SELECT [c].[Id], [c].[Model], [c].[OwnerId]
      FROM [Cars] AS [c]
      WHERE [c].[OwnerId] IS NULL

Which is checking against null, even though null OwnerIds have been converted to zero in the database.

Full repro:

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Migrations;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;

public static class ConvertNullsSample
{
    public static void Main()
    {
        Console.WriteLine();

        Helpers.RecreateCleanDatabase();

        using (var context = new CarsContext())
        {
            context.AddRange(
                new Car
                {
                    Model = "Toyota Yaris",
                    Owner = new() { Name = "Wendy" }
                },
                new Car
                {
                    Model = "Kia Soul"
                });

            context.SaveChanges();
            
            Console.WriteLine();
        }
        
        using (var context = new CarsContext())
        {
            // Not currently working
            var cars = context.Cars.Where(e => e.OwnerId == null).ToList();

            Console.WriteLine();

            foreach (var car in cars)
            {
                Console.WriteLine($"The {car.Model} does not have an owner.");
            }
        }
        
        Console.WriteLine();
    }

    public static class Helpers
    {
        public static void RecreateCleanDatabase()
        {
            using (var context = new CarsContext(quiet: true))
            {
                context.Database.EnsureDeleted();
                context.Database.Migrate();
            }
        }
    }

    #region PersonAndCar
    public class Person
    {
        public int Id { get; set; }
        public string Name { get; set; }

        public ICollection<Car> Cars { get; set; }
    }

    public class Car
    {
        public int Id { get; set; }
        public string Model { get; set; }
        
        public int? OwnerId { get; set; }
        public Person Owner { get; set; }
    }
    #endregion

    public class ZeroToNullConverter : ValueConverter<int?, int>
    {
        public ZeroToNullConverter()
            : base(
                v => v ?? 0,
                v => v == 0 ? null : v,
                convertsNulls: true)
        {
        }
    }

    public class CarsContext : DbContext
    {
        private readonly bool _quiet;

        public CarsContext(bool quiet = false)
        {
            _quiet = quiet;
        }

        public DbSet<Car> Cars { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder
                .EnableSensitiveDataLogging()
                .UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EFCoreSample");

            if (!_quiet)
            {
                optionsBuilder.LogTo(Console.WriteLine, new[] { RelationalEventId.CommandExecuted });
            }
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder
                .Entity<Car>()
                .Property(e => e.OwnerId)
                .HasConversion<ZeroToNullConverter>();
        }
    }
}

namespace CarsMigrations
{
    [DbContext(typeof(ConvertNullsSample.CarsContext))]
    [Migration("20210927174004_Cars")]
    public partial class Cars : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.CreateTable(
                name: "Person",
                columns: table => new
                {
                    Id = table.Column<int>(type: "int", nullable: false)
                        .Annotation("SqlServer:Identity", "1, 1"),
                    Name = table.Column<string>(type: "nvarchar(max)", nullable: true)
                },
                constraints: table =>
                    {
                        table.PrimaryKey("PK_Person", x => x.Id);
                    });

            migrationBuilder.CreateTable(
                name: "Cars",
                columns: table => new
                {
                    Id = table.Column<int>(type: "int", nullable: false)
                        .Annotation("SqlServer:Identity", "1, 1"),
                    Model = table.Column<string>(type: "nvarchar(max)", nullable: true),
                    OwnerId = table.Column<int>(type: "int", nullable: true)
                },
                constraints: table =>
                    {
                        table.PrimaryKey("PK_Cars", x => x.Id);
                    });

            migrationBuilder.CreateIndex(
                name: "IX_Cars_OwnerId",
                table: "Cars",
                column: "OwnerId");
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropTable(
                name: "Cars");

            migrationBuilder.DropTable(
                name: "Person");
        }
    }

    [DbContext(typeof(ConvertNullsSample.CarsContext))]
    partial class CarsContextModelSnapshot : ModelSnapshot
    {
        protected override void BuildModel(ModelBuilder modelBuilder)
        {
#pragma warning disable 612, 618
            modelBuilder
                .HasAnnotation("ProductVersion", "6.0.0-rc.1.21452.10")
                .HasAnnotation("Relational:MaxIdentifierLength", 128);

            SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder, 1L, 1);

            modelBuilder.Entity("ConvertNullsSample+Car", b =>
                {
                    b.Property<int>("Id")
                        .ValueGeneratedOnAdd()
                        .HasColumnType("int");

                    SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<int>("Id"), 1L, 1);

                    b.Property<string>("Model")
                        .HasColumnType("nvarchar(max)");

                    b.Property<int?>("OwnerId")
                        .HasColumnType("int");

                    b.HasKey("Id");

                    b.HasIndex("OwnerId");

                    b.ToTable("Cars");
                });

            modelBuilder.Entity("ConvertNullsSample+Person", b =>
                {
                    b.Property<int>("Id")
                        .ValueGeneratedOnAdd()
                        .HasColumnType("int");

                    SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<int>("Id"), 1L, 1);

                    b.Property<string>("Name")
                        .HasColumnType("nvarchar(max)");

                    b.HasKey("Id");

                    b.ToTable("Person");
                });

            modelBuilder.Entity("ConvertNullsSample+Car", b =>
                {
                    b.HasOne("ConvertNullsSample+Person", "Owner")
                        .WithMany("Cars")
                        .HasForeignKey("OwnerId");

                    b.Navigation("Owner");
                });

            modelBuilder.Entity("ConvertNullsSample+Person", b =>
                {
                    b.Navigation("Cars");
                });
#pragma warning restore 612, 618
        }
    }
}

[smitpatel]

Same as the other comment!

[ajcvickers]

Backlog for now. See also #26230.