Efcore: NullReferenceException when using QueryFilter filtering on owned types

[JesperTreetop]

This line causes an exception: https://github.com/aspnet/EntityFrameworkCore/blob/e4f00d4ae9871d1cb16b75af98cd12f068d3d7df/src/EFCore.Relational/Query/ExpressionVisitors/RelationalEntityQueryableExpressionVisitor.cs#L154

...when using QueryFilter on a property in two nested owned types.

Exception message: System.ArgumentNullException: 'Value cannot be null.'
Stack trace:    at Microsoft.EntityFrameworkCore.Utilities.Check.NotNull[T](T value, String parameterName)
   at Microsoft.EntityFrameworkCore.RelationalMetadataExtensions.Relational(IEntityType entityType)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.RelationalEntityQueryableExpressionVisitor.VisitEntityQueryable(Type elementType)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.EntityQueryableExpressionVisitor.VisitConstant(ConstantExpression constantExpression)
   at System.Linq.Expressions.ConstantExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.ReplaceClauseReferences(Expression expression, IQuerySource querySource, Boolean inProjection)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CompileGroupJoinInnerSequenceExpression(GroupJoinClause groupJoinClause, QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.CompileGroupJoinInnerSequenceExpression(GroupJoinClause groupJoinClause, QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.VisitGroupJoinClause(GroupJoinClause groupJoinClause, QueryModel queryModel, Int32 index)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitGroupJoinClause(GroupJoinClause groupJoinClause, QueryModel queryModel, Int32 index)
   at Remotion.Linq.Clauses.GroupJoinClause.Accept(IQueryModelVisitor visitor, QueryModel queryModel, Int32 index)
   at Remotion.Linq.QueryModelVisitorBase.VisitBodyClauses(ObservableCollection`1 bodyClauses, QueryModel queryModel)
   at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateQueryExecutor[TResult](QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](Expression query, IQueryModelGenerator queryModelGenerator, IDatabase database, IDiagnosticsLogger`1 logger, Type contextType)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass13_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at System.Linq.Queryable.Any[TSource](IQueryable`1 source)
   at EF21OwnedTypes.Program.Main(String[] args) in c:\EF21OwnedTypes\EF21OwnedTypes\Program.cs:line 28

Steps to reproduce

1. Create an empty .NET Core 2.1 console project.
2. Add these references:

  <ItemGroup>
    <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="2.1.0" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="2.1.0" />
  </ItemGroup>

1. Run Add-Migration to create the migration.
2. Run and watch it crash at !!A.
3. Comment out the line after !!B and it works.

```c#

define HASENTRIES

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Design;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace EF21OwnedTypes
{
class Program
{
static void Main(string[] args) {
var factory = new ApplicationDbContextFactory();
var ctx = factory.CreateDbContext(new[] {""});

        ctx.Database.EnsureDeleted();
        ctx.Database.Migrate();

        // !!!
        // !!A  This will blow up with NullReferenceException

        var any = ctx.MacGuffinDefinitions.Any();

        // !/A
        // !!!
    }
}


public class ApplicationDbContextFactory : IDesignTimeDbContextFactory<ApplicationDbContext>
{
    public ApplicationDbContext CreateDbContext(string[] args)
    {
        var optionsBuilder = new DbContextOptionsBuilder<ApplicationDbContext>();
        optionsBuilder.UseSqlite("Data Source=db.db");
        return new ApplicationDbContext(optionsBuilder.Options);
    }
}


public class ApplicationDbContext : DbContext
{
    public ApplicationDbContext(DbContextOptions<ApplicationDbContext> options)
        : base(options)
    {
    }

    public virtual DbSet<MacGuffinDefinition> MacGuffinDefinitions { get; set; }

if HASENTRIES

    public virtual DbSet<MacGuffinDefinitionHistory> MacGuffinDefinitionHistories { get; set; }

endif

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);

        // !!!
        // !!B comment out the following line and everything works:

        modelBuilder.Entity<MacGuffinDefinition>().HasQueryFilter(md => md.ChangeInfo.RemovedPoint.Timestamp == null);

        // !/B
        // !!!


        void HasGuidKey<TEntity>(EntityTypeBuilder<TEntity> entity, Expression<Func<TEntity, object>> keyExpression) where TEntity : class
        {
            entity.HasKey(keyExpression);
            entity.Property(keyExpression).HasDefaultValueSql("(newid())");
        }

        modelBuilder.Entity<MacGuffinDefinition>(entity => {
            HasGuidKey(entity, e => e.MacGuffinDefinitionID);

            entity.OwnsOne(m => m.ChangeInfo, ci => {
                ci.OwnsOne(mci => mci.RemovedPoint);
            });

if HASENTRIES

            entity.HasOne<MacGuffinDefinitionHistory>(h => h.LatestHistoryEntry).WithMany()
                .HasForeignKey(m => m.LatestHistoryEntryID).OnDelete(DeleteBehavior.SetNull);
            entity.HasMany<MacGuffinDefinitionHistory>(h => h.HistoryEntries).WithOne(h => h.MacGuffinDefinition)
                .HasForeignKey(h => h.MacGuffinDefinitionID).OnDelete(DeleteBehavior.Restrict);

endif

        });

if HASENTRIES

        modelBuilder.Entity<MacGuffinDefinitionHistory>(entity => {
            HasGuidKey(entity, e => e.MacGuffinDefinitionHistoryID);
            entity.Property(mdh => mdh.Name);

            entity.Property(h => h.IsLatest);

            entity.OwnsOne(h => h.CreatedPoint);
            entity.OwnsOne(h => h.EndedPoint);
        });

endif

    }

}


[Owned]
public class ChangePoint
{

    public ChangePoint()
    {
        IPAddress = "";
    }


    public DateTime Timestamp { get; set; }
    [Required]
    public string IPAddress { get; set; }
}

[Owned]
public class OptionalChangePoint
{
    public OptionalChangePoint(ChangePoint changePoint)
    {
        Timestamp = changePoint.Timestamp;
        IPAddress = changePoint.IPAddress;
    }

    public OptionalChangePoint()
    {

    }

    public DateTime? Timestamp { get; set; }
    public string IPAddress { get; set; }
}


[Owned]
public class MasterChangeInfo
{
    public DateTime CreatedTimestamp { get; set; }
    public DateTime ModifiedTimestamp { get; set; }

    public virtual OptionalChangePoint RemovedPoint { get; set; }

    public MasterChangeInfo()
    {

    }
}

if HASENTRIES

public partial class MacGuffinDefinitionHistory
{
    public Guid MacGuffinDefinitionHistoryID { get; set; }

    public Guid MacGuffinDefinitionID { get; set; }
    public virtual MacGuffinDefinition MacGuffinDefinition { get; set; }

    [Required]
    public string Name { get; set; }

    public ChangePoint CreatedPoint { get; set; }
    public OptionalChangePoint EndedPoint { get; set; }
    public bool IsLatest { get; set; }
}

endif

public partial class MacGuffinDefinition
{

if HASENTRIES

    public MacGuffinDefinition()
    {
        HistoryEntries = new HashSet<MacGuffinDefinitionHistory>();
    }

endif

    public Guid MacGuffinDefinitionID { get; set; }

    public virtual MasterChangeInfo ChangeInfo { get; set; }

if HASENTRIES

    public virtual ICollection<MacGuffinDefinitionHistory> HistoryEntries { get; set; }
    public virtual MacGuffinDefinitionHistory LatestHistoryEntry { get; set; }

    public Guid? LatestHistoryEntryID { get; set; }

endif

}

}
```

Further technical details

EF Core version: 2.1.0
Database Provider: any (tested with Microsoft.EntityFrameworkCore.SqlServer and Sqlite)
Operating system: Windows 10
IDE: Visual Studio 2017 15.7

[JesperTreetop]

I collapsed some of the metaprogramming in the first reproducible sample to make this clearer and to prove it wasn't related to those things.

[smitpatel]

A simplified repro would involve query filter defined on owned nav. (May need 2 levels).

[JesperTreetop]

Stopping at the stack frame referred to by line number at the top reveals that the entity type is MasterChangeInfo which is one of the owned types.

[JesperTreetop]

@smitpatel Updated with simplified repro code.

The seemingly irrelevant related entity seems to cause the condition somehow - uncomment #define HASENTRIES at the top of the sample repro, remake the migration and the problem goes away, even though the query filter doesn't touch the related entity!

Additionally, touching properties not two levels deep does not seem to cause the problem.

[ajcvickers]

@smitpatel Regression from 2.0?

[smitpatel]

Not a regression
2.0 did not allow to use any navigations inside query filters. It was blocked in model validation.

[JesperTreetop]

I think this issue is causing other issues (or is caused by something that's also causing other issues).

In the project that I pared down for this repro, making some further changes result in migrations being generated where the equivalent of this line is included in the Up method: migrationBuilder.DropColumn(name: "ChangeInfo_RemovedPoint_MasterChangeInfoMacGuffinDefinitionID", table: "MacGuffinDefinitions"); . In other words, I understand that the part in the snapshot class where the ID of the owner entity of the owned type is being mapped up as a property is some sort of glue that makes owned types work without breaking entity invariants. But a) it's being mapped up in the snapshot as a column that isn't being generated by the script, and b) this column is then being deleted in the generated migration, which of course causes a script error and stops everything.

I have been unable to make this issue appear in the repro project so far, and I can't make it not appear in the original project. I am open to submitting the codebase as-is and/or providing as detailed information as is necessary in private if it will help diagnose the issue. (I have made my email address visible in my Github profile.)

[maumar]

fixed in 1b96aa18a4b22bcee8b67a9802ee6116f30d5c25