efcore icon indicating copy to clipboard operation
efcore copied to clipboard

Access to a collection on an entity in Added state with lazy loading enabled

Open ansquero opened this issue 1 year ago • 5 comments

I've just activated lazy loading on our entities, which all have auto-incremented keys. I notice that on an instance of an entity added to the DbSet (and not yet saved), if I access a collection, a query is executed with a filter on a temporary id :

SELECT o."Id", o."OrderId", o."Price", o."ProductId", o."Quantity", o.xmin
FROM "OrderDetail" AS o
WHERE o."OrderId" = @__p_0 /* @__p_0 = -2147482646 */

This request will never return anything. Is this the desired behaviour for this request to be executed and for the collection not to be considered as loaded?

Include your code

The code that produces the previous query:

Order order = new();
order.Details.Add(new OrderDetail()); // This line has no impact on the triggering of the SQL query, so it could be deleted.
dbContext.GetDbSet<Order>().Add(order);
foreach (OrderDetail detail in order.Details) // <-- This line triggers a query
{
    detail.Quantity = 1;
}

The Order configuration:

builder.ToTable("Order");
builder.HasKey(e => e.Id).HasName("PK_ORDER");
builder.Property(p => p.CurrencyCode)
    .IsRequired();
builder.Property(p => p.CustomerId)
    .IsRequired();
builder.Property(p => p.Id)
    .ValueGeneratedOnAdd()
    .IsRequired();
builder.Property(p => p.OrderNumber)
    .IsRequired();
builder.HasOne(e => e.Customer).WithMany(e => e!.Orders).HasForeignKey(e => e.CustomerId).HasConstraintName("FK_Order_Customer").OnDelete(DeleteBehavior.ClientCascade);
builder.Navigation(e => e.Customer).HasField("_customer");
builder.HasMany(e => e.Details);
builder.Navigation(e => e.Details).HasField("_details");

The OrderDetail configuration:

builder.ToTable("OrderDetail");
builder.HasKey(e => e.Id).HasName("PK_ORDERDETAIL");
builder.Property(p => p.Id)
    .ValueGeneratedOnAdd()
    .IsRequired();
builder.Property(p => p.OrderId)
    .IsRequired();
builder.Property(p => p.Price)
    .IsRequired()
    .HasPrecision(19, 6);
builder.Property(p => p.ProductId)
    .IsRequired();
builder.Property(p => p.Quantity)
    .IsRequired();
builder.HasOne(e => e.Order).WithMany(e => e!.Details).HasForeignKey(e => e.OrderId).HasConstraintName("FK_OrderDetail_Order").OnDelete(DeleteBehavior.ClientCascade);
builder.Navigation(e => e.Order).HasField("_order");
builder.HasOne(e => e.Product).WithMany(e => e!.OrderDetails).HasForeignKey(e => e.ProductId).HasConstraintName("FK_OrderDetail_Product").OnDelete(DeleteBehavior.ClientCascade);
builder.Navigation(e => e.Product).HasField("_product");
builder.HasIndex(e => e.OrderId).HasDatabaseName("IDX_Order");

The Order entity:

public class Order : BusinessEntity
{
    private Customer _customer = null!;
    private ICollection<OrderDetail> _details = new System.Collections.Generic.List<EfLazyLoading.Domain.Entities.OrderDetail>();

    public Order()
    {
    }

    private Order(ILazyLoader lazyLoader)
        : base(lazyLoader)
    {
    }

    public string CurrencyCode { get; set; } = string.Empty;

    public int CustomerId { get; set; } = 0;

    public int Id { get; set; } = 0;

    public string? Notes { get; set; }

    public string OrderNumber { get; set; } = string.Empty;

    public Customer Customer
    {
        get { return LazyLoader.Load(this, ref _customer!)!; }
        set { _customer = value; }
    }

    [ForeignKey("OrderId")]
    public ICollection<OrderDetail> Details
    {
        get { return LazyLoader.Load(this, ref _details!)!; }
    }
}

The OrderDetail entity:

public class OrderDetail : BusinessEntity, IValidableEntity
{
    private Order _order = null!;
    private Product _product = null!;

    public OrderDetail()
    {
    }

    private OrderDetail(ILazyLoader lazyLoader)
        : base(lazyLoader)
    {
    }

    public int Id { get; set; } = 0;

    public int OrderId { get; set; } = 0;

    public decimal Price { get; set; } = 0.0M;

    public int ProductId { get; set; } = 0;

    public int Quantity { get; set; } = 1;

    public Order Order
    {
        get { return LazyLoader.Load(this, ref _order!)!; }
        set { _order = value; }
    }

    public Product Product
    {
        get { return LazyLoader.Load(this, ref _product!)!; }
        set { _product = value; }
    }
}

Include provider and version information

EF Core version: 7.21.9 Database provider: Npgsql.EntityFrameworkCore.PostgreSQL Target framework: .NET 7.0 Operating system: Windows 11 IDE: Visual Studio 2022 Version 17.7.3

ansquero avatar Feb 13 '24 14:02 ansquero

@ansquero This is a downside of lazy-loading proxies. That is, the database can be queried when it should not be because there is now no difference between accessing a property just to see the contents and accessing a property to load related data.

Note for team: this is currently by-design. We could throw for queries that use temporary values. Note that we have previously discussed not loading from Added entities, but this is perfectly valid if the FK is known, and is quite frequently used.

ajcvickers avatar Feb 13 '24 18:02 ajcvickers

Thank you @ajcvickers for the quick response.

I'm not sure I understood the second part of your answer. Do you mean that this could be a possible evolution (to no longer run the query in this case if the entity is in the added state and there is a FK in the database)? On further reflection, it doesn't matter whether the key is temporary or not, if an FK exists, I can't see any case where it's relevant to run a query when accessing a collection type navigation property on an entity in the added state.

In the current version (7 or 8), do you see a way of preventing this query from being triggered without affecting my entity classes?

Thanks

ansquero avatar Feb 14 '24 08:02 ansquero

if an FK exists, I can't see any case where it's relevant to run a query when accessing a collection type navigation property on an entity in the added state.

Consider a project with a CategoryId. It's entirely reasonable that a new project be associated with an existing category, and also it reasonable to load it when asked.

In the current version (7 or 8), do you see a way of preventing this query from being triggered without affecting my entity classes?

The typical way to deal with this is to not create proxies for new entities. This means newly created entities will not have lazy-loading capabilities. In fact, the more common question here is how to get lazy-loading to work in this case. Since you're not using proxies, consider adding a flag or some other mechanism to your entities so that they know when to load navigations or not.

Note that these are some of the very reasons that many consider lazy-loading to be an anti-pattern.

ajcvickers avatar Feb 14 '24 09:02 ajcvickers

if an FK exists, I can't see any case where it's relevant to run a query when accessing a collection type navigation property on an entity in the added state.

Consider a project with a CategoryId. It's entirely reasonable that a new project be associated with an existing category, and also it reasonable to load it when asked.

Yes but in your case Category is a reference navigation property. With a collection navigation property with a FK, I really can't think of any cases where that makes sense.

Note that these are some of the very reasons that many consider lazy-loading to be an anti-pattern.

Yes, we had quite a long debate about whether or not to activate it. We chose to activate it to secure our business code (to avoid incorrect behaviour if we browse a collection that hasn't been loaded).

ansquero avatar Feb 14 '24 10:02 ansquero

I think I've found an acceptable solution to our problem. As you suggested, I set a boolean in our base class to indicate whether the instance was created manually or by EF Core (following a read from the database). This boolean allows me to apply lazy loading to collections only when the instance exists in the database.

My base class:

public abstract class BusinessEntity
{
    protected BusinessEntity()
    {
    }

    protected BusinessEntity(ILazyLoader lazyLoader)
    {
        LazyLoader = lazyLoader;
        FromPersistenceLayer = true;
    }
    protected ILazyLoader? LazyLoader { get; private set; }

    private bool FromPersistenceLayer { get; } = false;

    protected TRelated GetRequiredReference<TRelated>(ref TRelated navigationField, [CallerMemberName] string navigationName = "")
        where TRelated : class
    {
        return GetReference(ref navigationField!, navigationName)!;
    }

    protected TRelated? GetReference<TRelated>(ref TRelated? navigationField, [CallerMemberName] string navigationName = "")
        where TRelated : class
    {
        return LazyLoader.Load(this, ref navigationField, navigationName);
    }

    protected TRelated GetCollection<TRelated>(ref TRelated navigationField, [CallerMemberName] string navigationName = "")
        where TRelated : class
    {
        return FromPersistenceLayer ? LazyLoader.Load(this, ref navigationField!, navigationName)! : navigationField;
    }
}

My modified entities:

public class Order : BusinessEntity
{
    private Customer _customer = null!;
    private ICollection<OrderDetail> _details = new System.Collections.Generic.List<EfLazyLoading.Domain.Entities.OrderDetail>();

    public Order()
    {
    }

    private Order(ILazyLoader lazyLoader)
        : base(lazyLoader)
    {
    }

    public string CurrencyCode { get; set; } = string.Empty;

    public int CustomerId { get; set; } = 0;

    public int Id { get; set; } = 0;

    public string? Notes { get; set; }

    public string OrderNumber { get; set; } = string.Empty;

    public Customer Customer
    {
        get { return GetRequiredReference(this, ref _customer); }
        set { _customer = value; }
    }

    [ForeignKey("OrderId")]
    public ICollection<OrderDetail> Details
    {
        get { return GetCollection(ref _details); }
    }
}
public class OrderDetail : BusinessEntity, IValidableEntity
{
    private Order _order = null!;
    private Product _product = null!;

    public OrderDetail()
    {
    }

    private OrderDetail(ILazyLoader lazyLoader)
        : base(lazyLoader)
    {
    }

    public int Id { get; set; } = 0;

    public int OrderId { get; set; } = 0;

    public decimal Price { get; set; } = 0.0M;

    public int ProductId { get; set; } = 0;

    public int Quantity { get; set; } = 1;

    public Order Order
    {
        get { return GetRequiredReference(ref _order); }
        set { _order = value; }
    }

    public Product Product
    {
        get { return GetRequiredReference(ref _product); }
        set { _product = value; }
    }
}

ansquero avatar Feb 14 '24 11:02 ansquero