Efcore: Calling static method in select and applying ToList produces wrong results.

Created on 24 Mar 2017  路  10Comments  路  Source: dotnet/efcore

Inconsistent behavior when calling extension method for projection (f.e. for mapping to DTO) with and without ToList. Looks like closure behavior. If call ToList on Select with static extension method call it produces duplicate values.

```c#
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.EntityFrameworkCore;

namespace TestEF
{
public class BloggingContext : DbContext
{
public DbSet Blogs { get; set; }
public DbSet Posts { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseInMemoryDatabase();

    //=> optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=WonkyVB;Trusted_Connection=True;");
}

public class Blog
{
    public int Id { get; set; }
    public string Title { get; set; }

    public ICollection<Post> Posts { get; set; }
}

public class Post
{
    public int Id { get; set; }
    public string Title { get; set; }

    public int? BlogId { get; set; }
    public Blog Blog { get; set; }
}

public class PostDTO
{
    public string Title { get; set; }
}

public static class Mappings
{
    public static PostDTO From(this PostDTO dto, Post post)
    {
        dto.Title = post.Title;
        return dto;
    }
}

public class Program
{
    public static void Main()
    {
        using (var context = new BloggingContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.Add(new Blog
            {
                Posts = new List<Post> { new Post { Title = "First" }, new Post { Title = "Second" }, new Post { Title = "Third" } }
            });
            context.SaveChanges();
        }

        using (var context = new BloggingContext())
        {
            var list = context.Posts.Select(p => new PostDTO().From(p)).ToList(); // Produces: Third Third Third
            //var list = context.Posts.Select(p => new PostDTO().From(p)); // Produces: First Second Third
            foreach (var item in list) Console.WriteLine(item.Title);
        }
    }
}

}
```

Further technical details

EF Core version: 1.1.1
Database Provider: (Microsoft.EntityFrameworkCore.InMemory, Microsoft.EntityFrameworkCore.SqlServer)
Operating system: Windows 10
IDE: (e.g. Visual Studio 2017)

closed-fixed type-bug

Most helpful comment

Re-opening this. Since we are removing ReLinq, we should look into supporting this kind of scenario. While this particular pattern may not be common, it is just a small subset of large scenario, in which user can call new class() which does not depend on range variable.

All 10 comments

I investigated into this and this is bug in ReLinq's QueryParser.

When we get the expression tree (after ParameterExtraction) the LambdaExpression in Select has NewExpression for PostDTO which converts to ConstantExpression of PostDTO in QueryModel.SelectClause after ReLinq parses the query.

Generated QueryModel.ToString

{from Post p in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests.QueryBugsTest+Post]) select value(Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests.QueryBugsTest+PostDTO).From([p])}

Point to mention that this does not happen if the new has any property assignment or anonymous type.

@MichaelKetting

@dmitryshunkov - As a workaround you can move defining new PostDTO from select clause. Effectively write query like this

var list = db.Posts.Select(p => From(p)).ToList();

public static PostDTO From(Post post)
{
    var dto = new PostDTO { Title = post.Title };
    return dto;
}

@smitpatel to file a relinq bug on this

@smitpatel Yes, an issue would be much approciated for bookkeeping.

PS: Sorry, I thought that I already responded here but looks like it got lost in the inbox queue. I try to do a repro for this in the re-linq tests over the weekend but the next couple of weeks will be crazy for me so I can't make promises and I'll apologize in advance for any delays :)

@smitpatel Just for bookkeeping: Thanks for the repro, it was a great tool for reminding me this can (and is meant to) be solved in EntityFramework using exisitng re-linq parsing infrastructure (IEvaluatableExpressionFilter). Please check the RMLNQ-112 issue for details.

Closing this for now since the pattern is not super common and there is no easy solution that would not negatively impact other scenarios. We would consider re-opening this based on feedback from others hitting it.

Re-opening this. Since we are removing ReLinq, we should look into supporting this kind of scenario. While this particular pattern may not be common, it is just a small subset of large scenario, in which user can call new class() which does not depend on range variable.

This may or may not work in 3.0 easily since even if Relinq is removed, our funcletizer may try to compute the value out for the initialization.

@smitpatel to provide workaround.

Was this page helpful?
0 / 5 - 0 ratings