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

LINQ projection gives inconsistent results using in memory database #20359

Closed
mateusz-szymanski opened this issue Mar 20, 2020 · 4 comments · Fixed by #24363
Closed

LINQ projection gives inconsistent results using in memory database #20359

mateusz-szymanski opened this issue Mar 20, 2020 · 4 comments · Fixed by #24363
Assignees
Labels
area-in-memory closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@mateusz-szymanski
Copy link

mateusz-szymanski commented Mar 20, 2020

Given certain configuration with relationships and owned types, linq on dbset gives inconsistent results using in memory database. Changing the order of selecting values in the projection makes a difference.

Steps to reproduce

I've created a simple solution with necessary setup and test: EFCoreIssuePlayground.zip (github).

...
            var result1 = (from r in dbContext.Root
                           select new
                           {
                               r.B.BValue,
                               r.A.Sub.AValue
                           }).FirstOrDefault();

            var result2 = (from r in dbContext.Root
                           select new
                           {
                               r.A.Sub.AValue,
                               r.B.BValue,
                           }).FirstOrDefault();

            Assert.Null(result1.BValue);
            Assert.NotNull(result2.BValue);

Further technical details

EF Core version: 3.1
Database provider: Microsoft.EntityFrameworkCore.InMemory, version: 3.1.2
Target framework: netstandard2.0
Operating system: Windows 10
IDE: Visual Studio 2019 16.4.3

@davidfon
Copy link

davidfon commented May 11, 2020

This issue is preventing me from upgrading from EF Core 2.2. I've attached another example using the latest 5.0 preview that demonstrates the issue with a slightly different setup. In the attached example, the selected properties become juxtaposed incorrectly depending on which projection is used (e.g. PersonFullName receives the value of entity.Person.PersonName.LastName).
EFCoreIssuePlayground.zip

@ajcvickers
Copy link
Contributor

@davidfon Presumably this is impacting test code only, since it's an in-memory database issue. To give us a rough idea of the impact, how many tests do you have, and how many of them are failing?

@davidfon
Copy link

@ajcvickers Sorry, retract the bit about not being able to upgrade. We can work around it by simply pulling the data with ToList() and then projecting after the fact. Still, it's a little scary seeing data being randomly assigned across the properties.

@mateusz-szymanski
Copy link
Author

@davidfon For the time being you could use another sql provider for tests, like sqLite. Then you can migrate to EF Core 3.0 and there is no need to artificially call ToList. It takes a bit longer to run the tests but aside from that it should work fine in most of the cases.

@ajcvickers ajcvickers modified the milestones: Backlog, 6.0.0 Nov 5, 2020
@smitpatel smitpatel assigned smitpatel and unassigned maumar Mar 9, 2021
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 9, 2021
smitpatel added a commit that referenced this issue Mar 10, 2021
Implement left join as client method to reduce complexity
To resolve the indexing issue stemming from #23934
Now all the projections are applied immediately to reshape the value buffer so that all our future bindings are always read value expressions which refers to a proper index.
In order to do this, we apply select for entity type with hierarchy to avoid entity check conditional expression.
For adding projection (through ReplaceProjectionMapping),
- For client projection, we apply a select and re-generate client projection as read values
- For projection mapping, we iterate the mappings, we apply a select and re-generate the mapping as read values
- If projection mapping is empty then we add a dummy 1 so that it becomes non-range-variable
When applying projection, we generate a selector lambda to form a value buffer and replaced all the expressions to read from new value buffer. Overall this solves the issue of having complex expressions to map or pull. This also removed PushDownIntoSubquery method.

In order to avoid the issue of indexes changing when generating join due to iterating projection mappings, we now also have projectionMappingExpressions which remembers the all expressions inside projectionMapping (which are all read value as we generated before). So now we don't need to iterate the mapping and we use the existing expressions directly. This keeps existing indexes.

Resolves #13561
Resolves #17539
Resolves #18194
Resolves #18435
Resolves #19344
Resolves #19469
Resolves #19667
Resolves #19742
Resolves #19967
Resolves #20359
Resolves #21677
Resolves #23360
Resolves #17537
Resolves #18394
Resolves #23934
smitpatel added a commit that referenced this issue Mar 10, 2021
Implement left join as client method to reduce complexity
To resolve the indexing issue stemming from #23934
Now all the projections are applied immediately to reshape the value buffer so that all our future bindings are always read value expressions which refers to a proper index.
In order to do this, we apply select for entity type with hierarchy to avoid entity check conditional expression.
For adding projection (through ReplaceProjectionMapping),
- For client projection, we apply a select and re-generate client projection as read values
- For projection mapping, we iterate the mappings, we apply a select and re-generate the mapping as read values
- If projection mapping is empty then we add a dummy 1 so that it becomes non-range-variable
When applying projection, we generate a selector lambda to form a value buffer and replace all the expressions to read from new value buffer. Overall this solves the issue of having complex expressions to map or pull. This also removed PushDownIntoSubquery method.

In order to avoid the issue of indexes changing when generating join due to iterating projection mappings, we now also have projectionMappingExpressions which remembers the all expressions inside projectionMapping (which are all read value as we generated before). So now we don't need to iterate the mapping and we use the existing expressions directly. This keeps existing indexes.

Resolves #13561
Resolves #17539
Resolves #18194
Resolves #18435
Resolves #19344
Resolves #19469
Resolves #19667
Resolves #19742
Resolves #19967
Resolves #20359
Resolves #21677
Resolves #23360
Resolves #17537
Resolves #18394
Resolves #23934
smitpatel added a commit that referenced this issue Mar 10, 2021
Implement left join as client method to reduce complexity
To resolve the indexing issue stemming from #23934
Now all the projections are applied immediately to reshape the value buffer so that all our future bindings are always read value expressions which refers to a proper index.
In order to do this, we apply select for entity type with hierarchy to avoid entity check conditional expression.
For adding projection (through ReplaceProjectionMapping),
- For client projection, we apply a select and re-generate client projection as read values
- For projection mapping, we iterate the mappings, we apply a select and re-generate the mapping as read values
- If projection mapping is empty then we add a dummy 1 so that it becomes non-range-variable
When applying projection, we generate a selector lambda to form a value buffer and replace all the expressions to read from new value buffer. Overall this solves the issue of having complex expressions to map or pull. This also removed PushDownIntoSubquery method.

In order to avoid the issue of indexes changing when generating join due to iterating projection mappings, we now also have projectionMappingExpressions which remembers the all expressions inside projectionMapping (which are all read value as we generated before). So now we don't need to iterate the mapping and we use the existing expressions directly. This keeps existing indexes.

Resolves #13561
Resolves #17539
Resolves #18194
Resolves #18435
Resolves #19344
Resolves #19469
Resolves #19667
Resolves #19742
Resolves #19967
Resolves #20359
Resolves #21677
Resolves #23360
Resolves #17537
Resolves #18394
Resolves #23934
Resolves #17620
Resolves #18912
smitpatel added a commit that referenced this issue Mar 10, 2021
Implement left join as client method to reduce complexity
To resolve the indexing issue stemming from #23934
Now all the projections are applied immediately to reshape the value buffer so that all our future bindings are always read value expressions which refers to a proper index.
In order to do this, we apply select for entity type with hierarchy to avoid entity check conditional expression.
For adding projection (through ReplaceProjectionMapping),
- For client projection, we apply a select and re-generate client projection as read values
- For projection mapping, we iterate the mappings, we apply a select and re-generate the mapping as read values
- If projection mapping is empty then we add a dummy 1 so that it becomes non-range-variable
When applying projection, we generate a selector lambda to form a value buffer and replace all the expressions to read from new value buffer. Overall this solves the issue of having complex expressions to map or pull. This also removed PushDownIntoSubquery method.

In order to avoid the issue of indexes changing when generating join due to iterating projection mappings, we now also have projectionMappingExpressions which remembers the all expressions inside projectionMapping (which are all read value as we generated before). So now we don't need to iterate the mapping and we use the existing expressions directly. This keeps existing indexes.

Resolves #13561
Resolves #17539
Resolves #18194
Resolves #18435
Resolves #19344
Resolves #19469
Resolves #19667
Resolves #19742
Resolves #19967
Resolves #20359
Resolves #21677
Resolves #23360
Resolves #17537
Resolves #18394
Resolves #23934
smitpatel added a commit that referenced this issue Mar 10, 2021
Implement left join as client method to reduce complexity
To resolve the indexing issue stemming from #23934
Now all the projections are applied immediately to reshape the value buffer so that all our future bindings are always read value expressions which refers to a proper index.
In order to do this, we apply select for entity type with hierarchy to avoid entity check conditional expression.
For adding projection (through ReplaceProjectionMapping),
- For client projection, we apply a select and re-generate client projection as read values
- For projection mapping, we iterate the mappings, we apply a select and re-generate the mapping as read values
- If projection mapping is empty then we add a dummy 1 so that it becomes non-range-variable
When applying projection, we generate a selector lambda to form a value buffer and replace all the expressions to read from new value buffer. Overall this solves the issue of having complex expressions to map or pull. This also removed PushDownIntoSubquery method.

In order to avoid the issue of indexes changing when generating join due to iterating projection mappings, we now also have projectionMappingExpressions which remembers the all expressions inside projectionMapping (which are all read value as we generated before). So now we don't need to iterate the mapping and we use the existing expressions directly. This keeps existing indexes.

Resolves #13561
Resolves #17539
Resolves #18194
Resolves #18435
Resolves #19344
Resolves #19469
Resolves #19667
Resolves #19742
Resolves #19967
Resolves #20359
Resolves #21677
Resolves #23360
Resolves #17537
Resolves #18394
Resolves #23934

# Conflicts:
#	test/EFCore.InMemory.FunctionalTests/Query/NorthwindGroupByQueryInMemoryTest.cs
@ghost ghost closed this as completed in #24363 Mar 10, 2021
ghost pushed a commit that referenced this issue Mar 10, 2021
Implement left join as client method to reduce complexity
To resolve the indexing issue stemming from #23934
Now all the projections are applied immediately to reshape the value buffer so that all our future bindings are always read value expressions which refers to a proper index.
In order to do this, we apply select for entity type with hierarchy to avoid entity check conditional expression.
For adding projection (through ReplaceProjectionMapping),
- For client projection, we apply a select and re-generate client projection as read values
- For projection mapping, we iterate the mappings, we apply a select and re-generate the mapping as read values
- If projection mapping is empty then we add a dummy 1 so that it becomes non-range-variable
When applying projection, we generate a selector lambda to form a value buffer and replace all the expressions to read from new value buffer. Overall this solves the issue of having complex expressions to map or pull. This also removed PushDownIntoSubquery method.

In order to avoid the issue of indexes changing when generating join due to iterating projection mappings, we now also have projectionMappingExpressions which remembers the all expressions inside projectionMapping (which are all read value as we generated before). So now we don't need to iterate the mapping and we use the existing expressions directly. This keeps existing indexes.

Resolves #13561
Resolves #17539
Resolves #18194
Resolves #18435
Resolves #19344
Resolves #19469
Resolves #19667
Resolves #19742
Resolves #19967
Resolves #20359
Resolves #21677
Resolves #23360
Resolves #17537
Resolves #18394
Resolves #23934

# Conflicts:
#	test/EFCore.InMemory.FunctionalTests/Query/NorthwindGroupByQueryInMemoryTest.cs
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-preview3 Mar 25, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-preview3, 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-in-memory closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants