Value conversion from null in the store generates bad queries

Open
#26,210 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Bug
Clarity
Needs clarification
Activity status
Stale
Tech stack
csharp, sql
Domain
databases

Research direction

Start by running the complete C# repro and comparing its generated SQL with the stored zero/null representation. Trace the query translation and value-conversion behavior; done would require an agreed expected result and a regression test covering the null comparison, but the issue does not name repository files or existing tests.

Written by the indexing model from the issue text.

Description

area-query area-type-mapping

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
        }
    }
}
Dominant language
C#
Stars
14.8k
Forks
3.4k
Avg merge
2d 5h
Merged PRs (30d)
134

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from dotnet/efcore

All issues in dotnet/efcore

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.