Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Initializing a non-entity ref type in select creates a single instance for every record #12274

Closed
kszicsillag opened this issue Jun 7, 2018 · 6 comments · Fixed by #22648
Closed
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@kszicsillag
Copy link

kszicsillag commented Jun 7, 2018

Consider the following entity type and its DTO type.

// Entity
public class Dog
{
    public int Id { get; set; }
    public string Name { get; set; }
}

//DTO
 public class DogDTO
 {
     public int ID { get; set; }
     public string  Name { get; set; }
     public IDCard IDCard { get; set; }
 }

 public class IDCard
 {
     public int ID { get; set; }
     public string Number { get; set; }
 }

Now query all the entities and project them to the DTO type.

 var dogDTOList = ctx.Dogs.Select(
   d => 
     new DogDTO { ID = d.Id
    ,Name = d.Name
    ,IDCard = new IDCard() })
.ToList();

We would expect that a new IDCard instance will be created for each DogDTO - as this is what would happen in case of LINQ-to-Objects.
However, the constructor of IDCard is called only once and the IDCard instance will be the same for all DogDTOs.

There is a full repro repo here.

Further technical details

EF Core version: 2.1.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows Server 2016
IDE: Visual Studio 2017 15.7

@kszicsillag
Copy link
Author

We observed that if the constructor is called with different parameter values for every record (e.g. with some data from the record) we get the expected beahvior: different instance created for every record.

@smitpatel
Copy link
Contributor

Duplicate of #7983

@smitpatel smitpatel marked this as a duplicate of #7983 Jun 7, 2018
@smitpatel
Copy link
Contributor

IDCard does not depend on the range variable hence being evaluated beforehand causing it to have same object for each result. There is no easy way to figure out this case.

@kszicsillag
Copy link
Author

kszicsillag commented Jun 8, 2018

Thanks for pointing me to #7983.
If this won't be fixed, at very least the documentation should warn about this issue, clearly state the limitations and give guidelines about how to write a projection without falling into this kind of pitfall.

@ajcvickers
Copy link
Contributor

Note from triage: we will reconsider this, perhaps pivoting on whether or no the call to new is in the final projection. Also, Relinq makes this problematic because it erases the New expression.

@ajcvickers ajcvickers added this to the Backlog milestone Jun 8, 2018
@smitpatel smitpatel added the verify-fixed This issue is likely fixed in new query pipeline. label Mar 15, 2020
@smitpatel
Copy link
Contributor

We have done work to now create parameter for NewExpression query tree. So this should likely be fixed.

@bricelam bricelam modified the milestones: Backlog, MQ Sep 11, 2020
maumar added a commit that referenced this issue Sep 21, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
maumar added a commit that referenced this issue Sep 21, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
maumar added a commit that referenced this issue Sep 21, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
@maumar maumar added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed verify-fixed This issue is likely fixed in new query pipeline. labels Sep 21, 2020
maumar added a commit that referenced this issue Sep 21, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
maumar added a commit that referenced this issue Sep 22, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
maumar added a commit that referenced this issue Sep 22, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
maumar added a commit that referenced this issue Sep 22, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
maumar added a commit that referenced this issue Sep 22, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
maumar added a commit that referenced this issue Sep 22, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
maumar added a commit that referenced this issue Sep 22, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
@ghost ghost closed this as completed in #22648 Sep 22, 2020
ghost pushed a commit that referenced this issue Sep 22, 2020
Resolves #12274
Resolves #11835
Resolves #12597
Resolves #12379
Resolves #12717
Resolves #17775
Resolves #18962
@ajcvickers ajcvickers modified the milestones: MQ, 6.0.0 Nov 25, 2020
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-preview1 Jan 27, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-preview1, 6.0.0 Nov 8, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants