Skip to content

ComplexType entities are null if Skip() is applied to an IQueryable that uses a static method for Select mapping. #18140

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
vegardlarsen opened this issue Sep 30, 2019 · 4 comments · Fixed by #19377

Comments

@vegardlarsen
Copy link

ComplexType entities are null if Skip() is applied to an IQueryable that uses a static method for Select mapping.

Steps to reproduce

async Task Main()
{
  var builder = new DbContextOptionsBuilder();
  
  // same results no matter which of these two providers you use
  //builder.UseSqlServer("Data Source=(localdb)\\MSSQLLocalDB;Initial Catalog=test;Integrated Security=True");
  builder.UseInMemoryDatabase("test");

  // setup consistent database state
  using (var context = new Context(builder.Options))
  {
    await context.Database.EnsureDeletedAsync();
    await context.Database.EnsureCreatedAsync();
    
    context.Tests.Add(new Test()
    {
      One = new ComplexTest { A = 42 }
    });
    
    await context.SaveChangesAsync();
  }  
  
  // test
  using (var context = new Context(builder.Options)) 
  {
    // create a queryable that maps the results using a static method
    var queryable = context.Tests.Select(t => Map(t));

    // Weird behavior starts here. 
    // If you run test case 1 first, it causes a NullReferenceException because the complex type
    // is null when the mapping happens. Since I can set a breakpoint and have it trigger in Map(), 
    // I assume this happens client-side in this case.
    // If you run test case 2 first (by commenting out test case 1), you get the desired result.
    // If you flip the order, and run 2 before 1, both of them gives you the correct result.
    // PS! .Dump calls are for comparing output in LINQpad.

    // Test case 1
    var skippedResults = await queryable.Skip(0).ToListAsync();
    skippedResults.Dump();

    // Test case 2
    var results = await queryable.ToListAsync();
    results.Dump();
  }
}

static int Map(Test x) {
  return x.One.A;
}

class Context : DbContext
{
  public Context(DbContextOptions options)
  : base(options)
  {
  }
  
  public DbSet<Test> Tests { get; set; }

  protected override void OnModelCreating(ModelBuilder modelBuilder)
  {
    base.OnModelCreating(modelBuilder);
    
    modelBuilder.Entity<Test>().OwnsOne(t => t.One);
  }
}

class Test 
{
  public int Id { get; set; }
  
  public ComplexTest One { get; set; }
}

[ComplexType]
class ComplexTest 
{
  public int A {get;set;}
}

I can't quite make heads or tails of this, but changing any of these things makes the problem disappear:

  • Using a lambda to do the mapping directly: .Select(t => t.One.A)
  • Reversing the order of the test cases.

Further technical details

EF Core version: 3.0.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer and Microsoft.EntityFrameworkCore.InMemory
Target framework: .NET Core 3.0
Operating system: Windows 10
IDE: LINQpad 6 (and Visual Studio 2019)

@smitpatel
Copy link
Contributor

Include seem to be ignored in first case.

@vegardlarsen
Copy link
Author

@smitpatel According to the list of breaking changes, there shouldn't have been any changes to how ComplexType works, and this was working in 2.2 (without any Include calls).

@roji
Copy link
Member

roji commented Oct 1, 2019

@vegardlarsen this is being investigated by @smitpatel and is probably a bug. The list of breaking changes only lists intentional breaking changes we made knowingly, but of course unintentional bugs slip through.

@ajcvickers ajcvickers added this to the 3.1.0 milestone Oct 7, 2019
@smitpatel
Copy link
Contributor

[ConditionalFact]
        public virtual void TEsting_issue_18140()
        {
            using (var context = CreateContext())
            {
                var query = context.Set<OwnedPerson>().Select(e => Map(e)).Skip(0).ToList();

            }
        }

        private static int Map(OwnedPerson p) => p.PersonAddress.Country.PlanetId;

@ajcvickers ajcvickers modified the milestones: 3.1.0, Backlog Oct 11, 2019
@smitpatel smitpatel modified the milestones: Backlog, 5.0.0 Dec 10, 2019
smitpatel added a commit that referenced this issue Dec 20, 2019
smitpatel added a commit that referenced this issue Dec 27, 2019
smitpatel added a commit that referenced this issue Dec 30, 2019
Resolves #18140
Resolves #18374
Resolves #18672
Resolves #18734
Resolves #19138
Resolves #19207
smitpatel added a commit that referenced this issue Dec 30, 2019
- Skip/Take does not force applying pending selector and changing shape.
- Throw translation failure message for Querayble methods which we don't translate (hence we don't process in navigation expansion). Earlier we threw query failed message. Now Navigation Expansion does not throw QueryFailed error message from any place.
- Unwrap type conversion for validating member access during include expansion so that we don't generate include when derived type's member is accessed.

Resolves #18140
Resolves #18374
Resolves #18672
Resolves #18734
Resolves #19138
Resolves #19207
smitpatel added a commit that referenced this issue Dec 30, 2019
- Skip/Take does not force applying pending selector and changing shape.
- Throw translation failure message for Querayble methods which we don't translate (hence we don't process in navigation expansion). Earlier we threw query failed message. Now Navigation Expansion does not throw QueryFailed error message from any place.
- Unwrap type conversion for validating member access during include expansion so that we don't generate include when derived type's member is accessed.

Resolves #18140
Resolves #18374
Resolves #18672
Resolves #18734
Resolves #19138
Resolves #19207
smitpatel added a commit that referenced this issue Jan 1, 2020
- Skip/Take does not force applying pending selector and changing shape.
- Throw translation failure message for Querayble methods which we don't translate (hence we don't process in navigation expansion). Earlier we threw query failed message. Now Navigation Expansion does not throw QueryFailed error message from any place.
- Unwrap type conversion for validating member access during include expansion so that we don't generate include when derived type's member is accessed.

Resolves #18140
Resolves #18374
Resolves #18672
Resolves #18734
Resolves #19138
Resolves #19207
smitpatel added a commit that referenced this issue Jan 1, 2020
- Skip/Take does not force applying pending selector and changing shape.
- Throw translation failure message for Querayble methods which we don't translate (hence we don't process in navigation expansion). Earlier we threw query failed message. Now Navigation Expansion does not throw QueryFailed error message from any place.
- Unwrap type conversion for validating member access during include expansion so that we don't generate include when derived type's member is accessed.

Resolves #18140
Resolves #18374
Resolves #18672
Resolves #18734
Resolves #19138
Resolves #19207
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview1 Mar 13, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview1, 5.0.0 Nov 7, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants