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: Identify defining query projection as entity type #17158

Open
smitpatel opened this issue Aug 14, 2019 · 4 comments
Open

Query: Identify defining query projection as entity type #17158

smitpatel opened this issue Aug 14, 2019 · 4 comments

Comments

@smitpatel
Copy link
Member

We treat new T { } as non-entity projection which we don't track and treat as entity shaper. Since defining query apply projection that way, we need to process it accordingly for include/shaper.

@smitpatel
Copy link
Member Author

This would allow us to apply Include on top of defining query or use defining query for keyed entity type where tracking is needed.

@divega
Copy link
Contributor

divega commented Aug 19, 2019

Some notes on the conversations I had with @smitpatel about this, in case they are useful in the future:

My understanding of this issue is that projecting an object with a new expression in a defining query makes it impossible to later navigate to related objects (e.g. include doesn't work) or track objects returned by the any queries based on such defining query.

Note that these are the same effects of projecting an object using new in a regular query, which we consider by-design.

Currently this will affect any key-less entity that references other key-less or keyed entities. If we re-enable defining query support for entity types (see #13358) this will affect keyed entity at the root as well.

We discussed a couple of options to address both the current and future incorrect behavior:

  1. Do nothing because essentially new in the defining query is no different from it in a regular LINQ query: it makes EF Core loose track of what object is supposed to mapped to if we track it, and what navigating its navigation property means.

  2. Throw an exception if a new expression is found in the projection of a defining query. Doing this after 3.0 will be a breaking change.

  3. Treat new expressions inside defining query differently from how we treat them outside, and automatically remap the object returned by the projection to the default mapping of its type.

@smitpatel
Copy link
Member Author

From an earlier discussion,
If ToTable/ToView, FromSql, Defining query is supposed to be equivalent in terms of how to get data for this DbSet from server, then we cannot do 0. It breaks our mapping assumption. Writing Include after FromSql is allowed so it should also be allowed after defining query.

I believe 2 is the best option (if we agree on above definition of mapping).
1 has low value. We disallow defining query on entity type so only issue comes is about doing Include, or EF.Property. We will throw at translation time. We would allow if you are not using those operators and query would work. (AsNoTracking & bindings work.)

@smitpatel
Copy link
Member Author

As per #18903 we want to replace current defining query with provider specific versions which avoids this issue altogether. We don't need to identify projection as entity type. In either provider it would be taken as a data mapping for the entity type rather than making the LINQ part of query expression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants