Simple query filter breaks simple projection

[ajcvickers]

As reported by @klaussj here: #12951 (comment)

Removing the query filter fixes the issue.

namespace DemoIssue
{
    public class Entity
    {
        public int Id { get; set; }
        public int? RefEntityId { get; set; }
        public RefEntity RefEntity { get; set; }
    }

    public class RefEntity
    {
        public int Id { get; set; }
        public bool Public { get; set; }
    }

    public class EntityDto
    {
        public int Id { get; set; }
        public int? RefEntityId { get; set; }
        public RefEntityDto RefEntity { get; set; }
    }

    public class RefEntityDto
    {
        public int Id { get; set; }
        public bool Public { get; set; }
    }

    public class DbTestContext : DbContext
    {
        public DbSet<Entity> Entities { get; set; }
        public DbSet<RefEntity> RefEntities { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<RefEntity>().HasQueryFilter(f => f.Public == true);
        }

        public DbTestContext(DbContextOptions<DbTestContext> options) : base(options)
        {

        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            using (var connection = new SqliteConnection("DataSource=:memory:"))
            {
                connection.Open();

                var options = new DbContextOptionsBuilder<DbTestContext>()
                    .UseSqlite(connection)
                    .Options;

                using (var db = new DbTestContext(options))
                {
                    db.Database.EnsureCreated();

                    db.RefEntities.Add(new RefEntity()
                    {
                        Id = 1,
                        Public = false
                    });

                    db.Entities.Add(new Entity()
                    {
                        Id = 1,
                        RefEntityId = 1
                    });

                    db.SaveChanges();

                    var notWorking = db.Entities.Select<Entity, EntityDto>(s =>
                        new EntityDto
                        {
                            Id = s.Id,
                            RefEntity = s.RefEntity == null ?
                                null :
                                new RefEntityDto()
                                {
                                    Id = s.RefEntity.Id,
                                    Public = s.RefEntity.Public
                                },
                            RefEntityId = s.RefEntityId
                        }).Single(p => p.Id == 1);
                }
            }
        }
    }
}

Exception:

Unhandled Exception: System.InvalidOperationException: Nullable object must have a value.
   at lambda_method(Closure , QueryContext , TransparentIdentifier`2 )
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ProjectionShaper.TypedProjectionShaper`3.Shape(QueryContext queryContext, ValueBuffer& valueBuffer)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.BufferlessMoveNext(DbContext _, Boolean buffer)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.MoveNext()
   at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ResultEnumerable`1.GetEnumerator()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Boolean& found)
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass15_1`1.<CompileQueryCore>b__0(QueryContext qc)
   at System.Linq.Queryable.Single[TSource](IQueryable`1 source, Expression`1 predicate)
   at DemoIssue.Program.Main(String[] args) in C:\Stuff\TwoOneCore\TwoOneCore\Program.cs:line 79

[ajcvickers]

@smitpatel to investigate

[smitpatel]

dbug: Microsoft.EntityFrameworkCore.Query[10101]
      Compiling query model:
      '(from Entity s in DbSet<Entity>
      where [s].Id == 1
      select new EntityDto{
          Id = [s].Id,
          RefEntity = [s].RefEntity == null ? null : new RefEntityDto{
              Id = [s].RefEntity.Id,
              Public = [s].RefEntity.Public
          }
          ,
          RefEntityId = [s].RefEntityId
      }
      ).Single()'
dbug: Microsoft.EntityFrameworkCore.Infrastructure[10407]
      'MyContext' disposed.
dbug: Microsoft.EntityFrameworkCore.Query[10104]
      Optimized query model:
      '(from Entity s in DbSet<Entity>
      join RefEntity s.RefEntity in
          from RefEntity f in DbSet<RefEntity>
          where [f].Public == True
          select [f]
      on Property([s], "RefEntityId") equals (Nullable<int>)Property([s.RefEntity], "Id") into s.RefEntity_group
      from RefEntity s.RefEntity in
          (from RefEntity s.RefEntity_groupItem in [s.RefEntity_group]
          select [s.RefEntity_groupItem]).DefaultIfEmpty()
      where [s].Id == 1
      select new EntityDto{
          Id = [s].Id,
          RefEntity = Property([s], "RefEntityId") == null ? null : new RefEntityDto{
              Id = (int)Property([s], "RefEntityId"),
              Public = (bool)?[s.RefEntity] | ?[s.RefEntity] | [s.RefEntity]?.Public == True? == True?
          }
          ,
          RefEntityId = [s].RefEntityId
      }
      ).Single()'
dbug: Microsoft.EntityFrameworkCore.Query[10107]
      (QueryContext queryContext) => IEnumerable<EntityDto> _InterceptExceptions(
      |__ source: IEnumerable<EntityDto> _ToSequence(() => EntityDto Single(IEnumerable<EntityDto> _ShapedQuery(
      |   |__ queryContext: queryContext,
      |   |__ shaperCommandContext: SelectExpression:
      |   |       SELECT TOP(2) [s].[Id], CASE
      |   |           WHEN [s].[RefEntityId] IS NULL
      |   |           THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)
      |   |       END, [s].[RefEntityId] AS [Id0], [t].[Public], [s].[RefEntityId]
      |   |       FROM [Entities] AS [s]
      |   |       LEFT JOIN (
      |   |           SELECT [f].*
      |   |           FROM [RefEntities] AS [f]
      |   |           WHERE [f].[Public] = 1
      |   |       ) AS [t] ON [s].[RefEntityId] = [t].[Id]
      |   |       WHERE [s].[Id] = 1,
      |   |__ shaper: (QueryContext queryContext | TransparentIdentifier<ValueBuffer, ValueBuffer> t1) => new EntityDto{
      |           Id = int TryReadValue(t1.Outer, 0, Entity.Id),
      |           RefEntity = (bool)Nullable<bool> TryReadValue(t1.Outer, 1, null) ? null : new RefEntityDto{
      |               Id = (int)Nullable<int> TryReadValue(t1.Outer, 2, Entity.RefEntityId),
      |               Public = (bool)Nullable<bool> TryReadValue(t1.Outer, 3, null)
      |           }
      |           ,
      |           RefEntityId = Nullable<int> TryReadValue(t1.Outer, 4, Entity.RefEntityId)
      |       }
      |__ ))),
      |__ contextType: EFSampleApp.MyContext,
      |__ logger: DiagnosticsLogger<Query>,
      |__ queryContext: Unhandled parameter: queryContext)

It is not query filter which is issue.
We generated Left Join here and there are no matching rows (due to filter), For some reason we are trying to read Null values in non-null properties even though there is a check.

Assigning to @maumar

[maumar]

problem is because we optimize [s].RefEntity == null into [s].RefEntity.Id == null and then using PK-FK optimization to '[s].RefEntityId == null`

The RefEntityId is not null, even when the entity is supposed to be filtered out with query filter. So the check:

RefEntity = s.RefEntity == null ?
                                null :
                                new RefEntityDto()
                                {
                                    Id = s.RefEntity.Id,
                                    Public = s.RefEntity.Public
                                }

doesn't really work, and we still try to materialize the RefEntityDto even though the RefEntity that we want to use has been filtered out.

Per discussion with @divega and @smitpatel, we should not apply the PK-FK optimization if we need to access the entity anyway somewhere else in the query. Optimization can save us a join, but if the join is there anyway, it doesn't buy us much.

[ajcvickers]

Notes from triage:

• As soon as a query filter is used on the principal table, then the relationship essentially becomes unconstrained--see Support "unconstrained" foreign key relationships #13146. This means that it is no longer correct to assume that a dependent has a corresponding principal.
• This means that optimizations that make this assumption cannot be used. For example, we cannot use an inner join, and we cannot assume that the FK value can be used instead of the PK value, since the PK value may be missing.
• We need better documentation on best practices on how how to use query filters. For example:
  • Relationships will be marked as loaded. Disabling or changing query filter parameters will not reset this flag
  • More generally, changing a filter parameter on the fly in a given context instance should only be done with great care or the results can be unexpected
  • Query filters should be applied appropriately so that principal and dependent queries will return consistent results.

[NetTecture]

What a triage. Instead of better documentation - did it ocur to you to issue a hotfix that fixes a bug that makes a whole subsystem (i.e. query filters) totally unusable? And it is nice to see that you basically tell all of us to go home and use EF - because a bug that shuts down a whole subsystem is scheduled for - ah, 3.0. Nice. Basically "oh, sorry you rely on query filterrs, maybe in a year we prouce a usable product there".

This is a 1.0 feature that does not work. 2.1.5 is the next patch. It should get this into a usable state. At minimum 2.2 should include an emergency fix for this. Otherwise basically EF is it - I do not care about EF Core because now you actually tell me that the new features that make it worthwhile and everyone loves pointing out - do not work and will not get fixed.

Any manager going through tirage and handing out reprimands? Bevcause this is the 2nd time this particular bug is either not accepted as bug (hey, fix your data) or - ah, scheudled for some far point in the future, usage and impact on users being irrelevant. I personally can only be happy that I am slowly removing query filters anyway (related to some business logic that makes them jsut the wrong place - i.e. we will filter on the REST side, not the model, as some related objects may be "oiutdated" but in our case then still are visible read only, so general filters do not work). But I can bet other people actually use that and are not happy about a 3.0 schedule.

[alpergunay]

Do you have any solution or workaround for this problem ?

[m17kea]

@ajcvickers @smitpatel @maumar I'm working on migrating a very large Fintech app from Net Framework to Net Core and this now completely blocking our company moving forward. Is there any chance you could produce a small fork with the work around or tell me where to remove the optimisation myself so that we can push on? What's the justification for leaving this until version 3, it seems to be pretty core functionality?

Many thanks for in advance.

[maumar]

@armitagemderivitec

This is the code that performs the optimization

https://github.com/aspnet/EntityFrameworkCore/blob/5964b5d159e31b18c69e1f8933616ef15296fb85/src/EFCore/Query/ExpressionVisitors/Internal/NavigationRewritingExpressionVisitor.cs#L732-L740

[m17kea]

@maumar that's great thanks! So I'd just need to fork and remove that block? Then the default would be used instead?

[maumar]

@armitagemderivitec yes, this will fix the case involving navigations (like in the original example). We will no longer try to convert entity.Navigation.Pk into entity.Fk.

[hisuwh]

Will calling .IgnoreQueryFilters() fix this?

[hisuwh]

That does seem to fix it. But I would like to add to other peoples sentiment that this is really a critical bug that should probably be patched before 3.0.0

[m17kea]

@maumar I've pulled EFCore locally and I don't even hit the code you mention above. The places where I am hitting this problem are all left joins where I am using DefaultIfEmpty() and then accessing properties on a potential nullable object. What's the correct form here because the select query does not present a nullable object?

[maumar]

The code I highlighted only addresses the case where EF is expanding navigation into a JOIN. If you create the joins yourself, and the query involves client evaluation you need to add your own null protection. You can do it using : ? operator, like so:

from c in ctx.Customers
join o in ctx.Orders on c.Name equal o.CustomerName into grouping
from o in grouping.DefaultIfEmpty()
select o != null ? (int?)o.InvoiceNumber : null

If the query is fully translated, the ? : part will be optimized out during the translation, so the SQL should look exactly the same with and without this part. However, if client evaluation is present it should prevent the error from happening.

What you might also be seeing is that one of the result properties is expected to be non-nullable, but it ends up as null.

You will get this error for queries like:

from c in ctx.Customers
join o in ctx.Orders on c.Name equal o.CustomerName into grouping
from o in grouping.DefaultIfEmpty()
select o.InvoiceNumber

result is expected to be of type int, but SQL returns a null value and we cant fit it into the expected result type.

[m17kea]

@maumar
This is one of our complex joins returning this error:

public static IQueryable<vBookUserAccess> vBookUserAccesses(this PortfolioDbContext context)
{
	return from ga in context.GroupAccesses
	   join g in context.Groups on new { ga.GroupId, GroupTypeId = 2 } equals
		   new { GroupId = g.Id, g.GroupTypeId }
	   join at in context.AccessTypes on ga.AccessTypeId equals at.Id
	   join gs in context.GroupStatuses on g.GroupStatusId equals gs.Id
	   join u in context.AspNetUsers on ga.UserId equals u.Id
	   join uas in context.UserAccountStatuses on u.AccountStatus equals uas.Id
	   join jr in context.Relationships() on ga.GroupId equals jr.ChildGroupId into joined
	   from relationship in joined.DefaultIfEmpty()
	   select relationship == null
		  ? new vBookUserAccess
				{
					Id = ga.GroupId,
					Name = g.Name,
					Description = g.Description,
					StatusId = g.GroupStatusId,
					StatusName = gs.Name,
					StatusDescription = gs.Description,
					NumberOfParentPortfolios = null,
					ParentPortfolioRelationshipTypeId = null,
					ParentPortfolioRelationshipTypeName = null,
					ParentPortfolioRelationshipTypeDescription = null,
					AccessUserId = ga.UserId,
					AccessUserName = u.UserName,
					AccessUserFirstName = u.FirstName,
					AccessUserLastName = u.LastName,
					AccessUserEmail = u.Email,
					AccessUserAccountStatusId = u.AccountStatus,
					AccessUserAccountStatusName = uas.Name,
					AccessUserAccountStatusDescription = uas.Description,
					AccessTypeId = ga.AccessTypeId,
					AccessTypeName = at.Name,
					AccessTypeDescription = at.Description,
					AccessCreatedDate = ga.CreatedDate,
					AccessCreatedBy = ga.CreatedBy,
					AccessModifiedDate = ga.ModifiedDate,
					AccessModifiedBy = ga.ModifiedBy,
					AccessVersion = ga.RowVersion
				}
		  : new vBookUserAccess
				{
					Id = ga.GroupId,
					Name = g.Name,
					Description = g.Description,
					StatusId = g.GroupStatusId,
					StatusName = gs.Name,
					StatusDescription = gs.Description,
					NumberOfParentPortfolios = relationship.NumberOfParentPortfolios,
					ParentPortfolioRelationshipTypeId =
						relationship.ParentPortfolioRelationshipTypeId,
					ParentPortfolioRelationshipTypeName =
						relationship.ParentPortfolioRelationshipTypeName,
					ParentPortfolioRelationshipTypeDescription =
						relationship.ParentPortfolioRelationshipTypeDescription,
			  
					AccessUserId = ga.UserId,
					AccessUserName = u.UserName,
					AccessUserFirstName = u.FirstName,
					AccessUserLastName = u.LastName,
					AccessUserEmail = u.Email,
					AccessUserAccountStatusId = u.AccountStatus,
					AccessUserAccountStatusName = uas.Name,
					AccessUserAccountStatusDescription = uas.Description,
					AccessTypeId = ga.AccessTypeId,
					AccessTypeName = at.Name,
					AccessTypeDescription = at.Description,
					AccessCreatedDate = ga.CreatedDate,
					AccessCreatedBy = ga.CreatedBy,
					AccessModifiedDate = ga.ModifiedDate,
					AccessModifiedBy = ga.ModifiedBy,
					AccessVersion = ga.RowVersion
				};
}

private static IQueryable<Relationship> Relationships(this PortfolioDbContext context)
{
	return from gr in context.GroupRelationships
	   join gs in context.Groups on new { gr.ParentGroupId, GroupTypeId = 1 } equals
		   new { ParentGroupId = gs.Id, gs.GroupTypeId }
	   join relationshipType in context.RelationshipTypes on gr.RelationshipTypeId equals
		   relationshipType.Id
	   group gr by new
		   {
			   gr.ChildGroupId,
			   gr.RelationshipTypeId,
			   relationshipType.Name,
			   relationshipType.Description
		   }
	   into g
	   select new Relationship
		  {
			  ChildGroupId = g.Key.ChildGroupId,
			  ParentPortfolioRelationshipTypeId = g.Key.RelationshipTypeId,
			  ParentPortfolioRelationshipTypeName = g.Key.Name,
			  ParentPortfolioRelationshipTypeDescription = g.Key.Description,
			  NumberOfParentPortfolios = g.Count()
		  };
}

private class Relationship
{
	public int ChildGroupId { get; set; }

	public int NumberOfParentPortfolios { get; set; }

	public string ParentPortfolioRelationshipTypeDescription { get; set; }

	public int ParentPortfolioRelationshipTypeId { get; set; }

	public string ParentPortfolioRelationshipTypeName { get; set; }
}

This worked in EF6 even without the null operator. Presumably the nested IQueryable is the cause. Any ideas?

[maumar]

@armitagemderivitec I filed the new bug - #14773 since this looks like a different issue, no QueryFilters seem to be involved in the scenario.

[ackava]

What a triage. Instead of better documentation - did it ocur to you to issue a hotfix that fixes a bug that makes a whole subsystem (i.e. query filters) totally unusable? And it is nice to see that you basically tell all of us to go home and use EF - because a bug that shuts down a whole subsystem is scheduled for - ah, 3.0. Nice. Basically "oh, sorry you rely on query filterrs, maybe in a year we prouce a usable product there".

This is a 1.0 feature that does not work. 2.1.5 is the next patch. It should get this into a usable state. At minimum 2.2 should include an emergency fix for this. Otherwise basically EF is it - I do not care about EF Core because now you actually tell me that the new features that make it worthwhile and everyone loves pointing out - do not work and will not get fixed.

Any manager going through tirage and handing out reprimands? Bevcause this is the 2nd time this particular bug is either not accepted as bug (hey, fix your data) or - ah, scheudled for some far point in the future, usage and impact on users being irrelevant. I personally can only be happy that I am slowly removing query filters anyway (related to some business logic that makes them jsut the wrong place - i.e. we will filter on the REST side, not the model, as some related objects may be "oiutdated" but in our case then still are visible read only, so general filters do not work). But I can bet other people actually use that and are not happy about a 3.0 schedule.

Its the manager who decides whether it is useful for marketing or not, problem is EF team has never taken problems seriously but are always busy in introducing new in built features of SQL Server or now Cosmos DB in EF Core. The whole focus is on Cosmos DB and all bugs are scheduled for future !!

Open source term is misleading here, there is no community and there is no control over what will happen next.

[MintPlayer]

I didn't have this issue in .NET Core 2.2, Now I'm using .NET Core 3.0 and I'm having trouble with this when using a QueryFilter.

I agree that complex filtering queries should not be run client-side but rather with SQL, which is way faster for large datasets.

But if the QueryFilter contains a simple expression (which it should), this still can be converted to SQL (can it?) and there's no problem...
My use case is the following:

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

	modelBuilder.Entity<Subject>().ToTable("Subjects");
	modelBuilder.Entity<Subject>().Property(s => s.Id).ValueGeneratedOnAdd();
	modelBuilder.Entity<Subject>().HasQueryFilter(s => s.UserDelete == null);
}

And just fetching an entity by id already fails:

var entity_person = mintplayer_context.People.Find(person.Id);

I'm a little troubled by this, because since some developers are using bad practices (yielding results from a potentially large DbSet before executing a linq query), now a decision is made that even affects the situation described above. However a collegue at work told me this could be happening while you're not aware of it...

But anyway, even the code above isn't working because of this.

[smitpatel]

@MusicDemons - Please file a new issue with repro code which we can run. Your issue does not seem to be related to this issue.

[NetTecture]

some developers are using bad practices (yielding results from a potentially large DbSet before
executing a linq query)

Given the state of EfCore query execution that is not bad practice - is it an approach that works. I do the same. I route ALL queries through a helper method. The helper method does use analyisis of the query parameters and the LINQ nodes to decide whether or not to offload the query to SQL. If not - all data is laoded and evaluated in memory.

Bad practice? NO. It is a workaround until I either get so tired to rip out EfCore, or they fix the bugs and I slowly take out cases where I need to evaluate locally.

[roji]

@NetTecture you're clearly frustrated with the state of things, but can you please refrain from posting comments that aren't relevant to the issue? The team is working extremely hard to address issues to the best of our abilities, and this doesn't help us make progress.