dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.74k stars 3.18k forks source link

Consolidate EntityType and QueryType #14194

Closed ajcvickers closed 5 years ago

ajcvickers commented 5 years ago

We introduced "query types" in 2.1 as types that don't have a primary key defined. However, more and more it is looking that these should just be entity types without primary keys defined. This doesn't mean that they will behave differently, just that they will be represented by and called entity types. This has the following advantages:

One thing to be careful of is that we don't treat something that should have a PK as something that doesn't have a PK by convention, since this will result in limited functionality (i.e. query type functionality) on something that is expected to have entity type functionality, which could be hard to debug. We might want to consider requiring something explicit like HasNoKey or an appropriate annotation to make one of these types.

ajcvickers commented 5 years ago

Note about query types. After further discussion, all navigations to query types will be disallowed. This is because:

ajcvickers commented 5 years ago

Note from planning: we will obsolete the current APIs since a hard break in 3.0 will be too painful.

NetTecture commented 5 years ago

@ajcvickers There is also a point of view that the world is flat. Makes no logical sense.

Here are scenarios of query types that are reachable from entity types. Views.

I have a lot of view based query types, mostly because the DB is not made by "code first, ignore all SQL level features" style. The ability to reference query types (i.e. views) from an entity type is critical in this scenario. And no, you can not express advanced view and sql thingies in LINQ. No way.

ajcvickers commented 5 years ago

@NetTecture If the views have unique column(s) that can act as a key, then map them to entity types.

Suchiman commented 5 years ago

@ajcvickers

That is, in a well-defined model query types should only ever have references to entity types.

Query types […] should never reference entity types, even if the query type has an FK property and there is an entity type with a PK that matches that FK.

Only one of these statements can be true, and i hope it's the first because i don't see why Query types should be constrained the second way. Makes them less useful and you now have to write manual joins...

AndriySvyryd commented 5 years ago

@Suchiman

That is, in a well-defined model query types should only ever have references to entity types.

Query types […] should never reference entity types, even if the query type has an FK property and there is an entity type with a PK that matches that FK.

Only one of these statements can be true, and i hope it's the first because i don't see why Query types should be constrained the second way. Makes them less useful and you now have to write manual joins...

I think @ajcvickers meant to say "Entity types should never reference query types, even if the query type has an FK property and there is an entity type with a PK that matches that FK."

NetTecture commented 5 years ago

And the reason is why?

I have a ton of entities that have query types - totally read only types - that are coming from a view attached.

Not entities by definition, referenced from an entity.

ajcvickers commented 5 years ago

@NetTecture Why can't those be mapped as entity types?

lightel commented 5 years ago

@NetTecture Why can't those be mapped as entity types?

Maybe I'm missing something so let me explain my case a bit more in details so that you could understand my confusion. We are building the API using an existing legacy database. So basically we use the Database First Approach. In a legacy database we have many views which returns ids of entities (some of them contain a column which can serve as PK). We used QueryType with EF Core 2.1 and mapped such entities to corresponding views. We also added navigation properties so that we could make joins to related entities. With EF Core 2.2 this doesn't work anymore. My understanding is that you are suggesting to pretend that these views are tables and map them to entities (with current implementation an entity can be mapped to table only). With the Code First Approach, I guess, this would work just fine because a migration would create a table instead of a view in a database. But with the Database First Approach once I mapped a view to an entity nothing prevents me (or any other person who sees my code for the first time) from assuming that it's a regular entity which is mapped to a regular table therefore it is pretty safe to insert a new record what will obviously cause a crash. Can you advise on this? I will appreciate any input.

ajcvickers commented 5 years ago

@lightel Just because the mapping is to a view doesn't necessarily mean that the returned objects don't have keys or are read-only. Likewise, just because the mapping is to a table doesn't necessarily mean that the returned objects must have keys and be updatable. Query types have one feature that defines them--they don't have keys defined. This doesn't mean they have to be used with views. Likewise entity types with keys defined don't have to be used with tables.

Scaffolding to views is covered here: #1679 Excluding types from migrations is covered here: #2725 Read-only entity types are covered here: #7586

JonPSmith commented 5 years ago

Can I add some thoughts here.

I agree that having a DbSet<T> and a DbQuery<T> had some down sides, especially in libraries where you needed to check if an entity was a DbSet<T> or a DbQuery<T>.

However I think the concept of a "Read-Only" entity class is a good one. I have build systems where I use multiple DBContext to create DDD type "bounded contexts" in one database. I use a DbQuery to enforce that one bounded context has access to, but cannot write to, a particular table. I can also see a technical reason for having a read-only entity class when a view is not updatable.

I looked at the Read-only entity types issue #7586, but its very old and doesn't seem to go anywhere. Is there something like add a fluent API method like .IsReadOnly a possibility?

roji commented 5 years ago

@JonPSmith agreed that a read-only entity type can definitely be useful, but note that as explained in #7586 it's relatively easy to achieve that without any special support from EF Core (or at least some version of that).

JonPSmith commented 5 years ago

Hi @roji, I saw that. It works but its not automatic. Also overriding SaveChanges and using ChangeTracker require some finesse to do properly and being efficient, i.e. not calling ChangeTracker twice. But I accept that might be the way I need to go if I want to enforce read-only.

ajcvickers commented 5 years ago

@JonPSmith Read-only types is something we intend to implement. The point is that query types were not ever intended to be used because they are read-only. That's just a side-effect of not having a key. This doesn't change when making them entity types without keys; they will still be read-only out of necessity.

KevinEarley-NCSos commented 5 years ago

Is there a trick to getting this to work? Latest preview I get the key required message.

AndriySvyryd commented 5 years ago

@kpejr You need to call ModelBuilder.Entity<MyEntity>().HasNoKey() See breaking changes

KevinEarley-NCSos commented 5 years ago

@AndriySvyryd Thanks, missed that working great now.

lbogdanov commented 5 years ago

@ajcvickers one of the query type's benefits - you can pre-define queries using raw sql in your code, no need to create a views in the database. Will this feature be retained in some form and shape in v3.0? modelBuilder.Query<MyQueryType>().ToQuery( () => Query<MyQueryType>().FromSql("select * from mytable")); Thank you

AndriySvyryd commented 5 years ago

@ibogdanov Yes, all functionality remains the same. You just need to call 'modelBuilder.Entity().HasNoKey()' instead of 'modelBuilder.Query()'