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.5k stars 3.13k forks source link

Create check constraints for columns that are only conditionally nullable #20931

Open pekspro opened 4 years ago

pekspro commented 4 years ago

I'm having an Order object, with StreetAddress as an owned entity.

public class Order
{
    public int Id { get; set; }
    public StreetAddress ShippingAddress { get; set; }
}

public class StreetAddress
{
    public int PostCode { get; set; }
    public string City { get; set; }
}

In OnModelCreating I have configured both PostCode and City to be required. But despite that I could add an Order without City and save it successfully into the database (I looks correct in the database too). But if I then read back that order ShippingAddress will be null.

Honestly, I am not sure what to expect in this scenario, but this look weird. If I cannot use required fields on owned entities, I will expect an exception if I try to do that. If it is supported, I should get an exception when I try to save properties with missing values.

Steps to reproduce

You could clone: https://github.com/pekspro/OwnedEntitiesTest

Or just run this code:

using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.Linq;

namespace EFModeling.OwnedEntities
{

    public class Order
    {
        public int Id { get; set; }
        public StreetAddress ShippingAddress { get; set; }
    }

    public class StreetAddress
    {
        public int PostCode { get; set; }
        public string City { get; set; }
    }

    public class OwnedEntityContext : DbContext
    {
        public DbSet<Order> Orders { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            => optionsBuilder
                .UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EFOwnedEntity;Trusted_Connection=True;ConnectRetryCount=0");

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Order>().OwnsOne(
                o => o.ShippingAddress,
                sa =>
                {
                    // NOTE: IsRequired() is used here
                    sa.Property(p => p.PostCode).HasColumnName("PostCode").IsRequired();
                    sa.Property(p => p.City).HasColumnName("ShipsToCity").IsRequired();
                });
        }
    }

    public static class Program
    {
        static void Main(string[] args)
        {
            using (var context = new OwnedEntityContext())
            {
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();

                context.Add(new Order
                {
                    ShippingAddress = new StreetAddress()
                    {
                        City = null,
                        PostCode = 12345
                    }
                });

                // Why does this works when City i required?
                context.SaveChanges();
            }

            using (var context = new OwnedEntityContext())
            {
                var orders = context.Orders.Include(a => a.ShippingAddress).ToList();

                foreach(var order in orders)
                {
                    // One order is printed here, but ShippingAddress is null even if we should get a value for PostCode.
                    Console.WriteLine($"OrderID: {order.Id} PostCode: {order.ShippingAddress?.PostCode} City: {order?.ShippingAddress?.City}");
                }
            }
        }
    }
}

Further technical details

EF Core version:3.1.3 Target framework: NET Core 3.1

Just for fun I tried with EF Core version 5.0.0-preview.3.20181.2 but it did behave the same.

ajcvickers commented 4 years ago

@AndriySvyryd @smitpatel I assume the columns are nullable here because of optional owned types?

@pekspro EF Core doesn't do any validation that the entities match the model you define, beyond that which is required to generate correct SQL. This is because doing so is considerable extra overhead in the general case which duplicates validation which must usually be done at higher levels anyway.

smitpatel commented 4 years ago

From query perspective, ShippingAddress is null because one of the required column has null value. How SaveChanges allowed that to be saved could be a bug.

pekspro commented 4 years ago

Thanks for a fast response, @ajcvickers and @smitpatel. The fact that EF Core does not validate on save explains a lot. But I am a bit puzzled about:

From query perspective, ShippingAddress is null because one of the required column has null value.

Does this mean that there is validation when data is read from the database? That is a bit unexpected.

Also, ShippingAddress itself is optional now in my model as I understand it. Is it possible to make this a required property? I would like to avoid that PostCode and City is nullable in the database.

smitpatel commented 4 years ago

See #12100

ajcvickers commented 4 years ago

We discussed the overall situation where a property is required in the model but the column in the database cannot be nullable. The most common case of this is for TPH, but it may also be applicable to table sharing scenarios. Even though the column can't be nullable, we know it can only be null in certain conditions--for example, when the discriminator column has certain values. We could generate check constraints in the database for this.

pekspro commented 4 years ago

Thanks for you are thinking about this, @ajcvickers . In my real-life scenario where I run into this, I changed my strategy to use base classes and interfaces instead. It was good enough for me :-)

AndriySvyryd commented 1 year ago

Related to https://github.com/dotnet/efcore/issues/2595