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

EntityEquality generates type mismatch #16564

Closed
smitpatel opened this issue Jul 11, 2019 · 4 comments · Fixed by #16957
Closed

EntityEquality generates type mismatch #16564

smitpatel opened this issue Jul 11, 2019 · 4 comments · Fixed by #16957
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@smitpatel
Copy link
Contributor

Test: Contains_over_entityType_with_null_should_rewrite_to_identity_equality

                var query
                    = context.Orders.Where(o => o.CustomerID == "VINET")
                        .Contains(null);
bool Contains(
|__ source: IQueryable<Order> Where(
|   |__ source: DbSet<Order>, 
|   |__ predicate: (Order o) => o.CustomerID == "VINET"), 
|__ item: (Unhandled parameter: __p_0))

// Gets rewritten into

bool Contains(
|__ source: IQueryable<int> Select(
|   |__ source: IQueryable<Order> Where(
|   |   |__ source: DbSet<Order>, 
|   |   |__ predicate: (Order Param_0) => Param_0.CustomerID == "VINET"), 
|   |__ selector: (Order Param_0) => Property(Param_0, "OrderID")), 
|__ item: (Unhandled parameter: __entity_equality_p_0_OrderID))

here p0 is null so runtime generated parameter is also null. But value is being cast to int which is NRE. When rewriting key access on potentially null expression, it should convert the types to nullable.

@smitpatel
Copy link
Contributor Author

smitpatel commented Jul 11, 2019

Another case

IQueryable<Customer> Where(
|__ source: DbSet<Customer>, 
|__ predicate: (Customer c) => Order FirstOrDefault(IOrderedEnumerable<Order> OrderBy(
    |__ source: c.Orders, 
    |__ keySelector: (Order o) => o.OrderID)) == Order 10243)

It generates property access of type int on FOD.
Which nav expansion after member pushdown generates this

IQueryable<Customer> Where(
|__ source: DbSet<Customer>, 
|__ predicate: (Customer Param_0) => (int)Nullable<int> FirstOrDefault(IQueryable<Nullable<int>> Select(
    |__ source: IOrderedQueryable<Order> OrderBy(
    |   |__ source: IQueryable<Order> Where(
    |   |   |__ source: DbSet<Order>, 
    |   |   |__ predicate: (Order o) =>  ?= Property(Param_0, "CustomerID") == Property(o, "CustomerID") =? ), 
    |   |__ keySelector: (Order Param_0) => Param_0.OrderID), 
    |__ selector: (Order Param_0) => (Nullable<int>)Property(Param_0, "OrderID"))) == 10243)

This throw NRE in InMemory.

@ajcvickers ajcvickers added this to the 3.0.0 milestone Jul 12, 2019
@roji
Copy link
Member

roji commented Jul 18, 2019

Sorry, my github email notifications are clearly not working well - missed this. Will check it out as soon as the preview7 port is done.

@roji
Copy link
Member

roji commented Jul 29, 2019

@smitpatel I see the issue you're describing. However, the first test (Contains_over_entityType_with_null_should_rewrite_to_identity_equality) passes correctly - do you have another repro in mind for relational?

Will investigate the second failure.

Regardless, will wait on #16791 to get merged to avoid conflicts.

@smitpatel
Copy link
Contributor Author

InMemory failure. The test is disabled for InMemory provider with this bug number.

roji added a commit that referenced this issue Aug 5, 2019
roji added a commit that referenced this issue Aug 5, 2019
roji added a commit that referenced this issue Aug 5, 2019
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 9, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview9 Aug 21, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview9, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants