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

Query: fix edge cases around Cast/OfType result operator lifting #8334

Closed
maumar opened this issue Apr 29, 2017 · 5 comments
Closed

Query: fix edge cases around Cast/OfType result operator lifting #8334

maumar opened this issue Apr 29, 2017 · 5 comments
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Apr 29, 2017

There are some issues around result operator lifting, specifically Cast and OfType:

1.) cast used as type validation doesn't always throw when it should, e.g.:

from g in ctx.Gears.Cast<Officer>().Cast<Gear>()
select g.Nickname

when run on the client it will throw, because not every gear is an officer. However it works fine on sql sever, even though during QM optimization we turn this to:

from g in ctx.Gears
select ((Gear)((Officer)g)).Nickname

2.) Before we try to lift cast, we handle OfType, by updating the type on EntityQueryable directly, e.g:

from o in (from g in EntityQueryable<Gear> select g).OfType<Officer>()
select o.NumberOfReports

will get converted to:

from o in EntityQueryable<Officer>
select o.NumberOfReports

This however breaks down if Cast is introduced to the mix:

from o in (from g in EntityQueryable<Gear> select g).Cast<Gear>().OfType<Officer>()
select o.NumberOfReports

will fail, because we process OfType first:

from o in (from g in EntityQueryable<Officer> select g).Cast<Gear>()
select o.NumberOfReports

which then gets lifted to:

from o in (from g in EntityQueryable<Officer> select g)
select ((Gear)o).NumberOfReports

which is wrong

@maumar
Copy link
Contributor Author

maumar commented Apr 29, 2017

2.) can be addressed by removing all Cast operators that come before the given OfType, but that potentially makes 1) worse

@ajcvickers ajcvickers added this to the Backlog milestone May 1, 2017
@ajcvickers
Copy link
Contributor

@maumar Can you write down some of the ideas we discussed in triage?

@maumar
Copy link
Contributor Author

maumar commented May 1, 2017

We should force client eval on Cast operator to mimic its type asserting functionality. However, some queries inject unnecessary casts, e.g.

from Customer c in ctx.Customers
select c.Name

we should strip casts for those cases (where the cast type is the same as element type of EntityQueryable

@smitpatel
Copy link
Contributor

  1. would be irrelevant now that we don't have client eval.

@smitpatel smitpatel self-assigned this Nov 21, 2019
@smitpatel smitpatel modified the milestones: Backlog, 3.1.0 Nov 21, 2019
@smitpatel smitpatel added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed try-on-latest labels Nov 21, 2019
smitpatel added a commit that referenced this issue Nov 21, 2019
@smitpatel
Copy link
Contributor

2nd case is fixed in 3.1 release.

smitpatel added a commit that referenced this issue Nov 22, 2019
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. type-bug
Projects
None yet
Development

No branches or pull requests

4 participants