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

Enable optional dependents when using table splitting #9005

Open
AndriySvyryd opened this Issue Jun 28, 2017 · 21 comments

Comments

Projects
None yet
9 participants
@AndriySvyryd
Member

AndriySvyryd commented Jun 28, 2017

One way of doing this could be mapping the dependent FK to non-PK columns

@julielerman

This comment has been minimized.

julielerman commented Jul 10, 2017

subscribed! :)

@divega

This comment has been minimized.

Member

divega commented Jul 10, 2017

Another way we could consider addressing this is by storing a sentinel value indicating if the object exists in a separate Boolean column. E.g. think Address_IsSet. I wonder if this already works as a workaround with the help of global query filters.

@JackHerring

This comment has been minimized.

JackHerring commented Aug 7, 2017

What is the status of this? As I think it is kind of a show stopper

@AndriySvyryd

This comment has been minimized.

Member

AndriySvyryd commented Aug 7, 2017

@JackHerring This is on the list of things we plan to do, but we don't have a specific version/date for when it would be available.

@JackHerring

This comment has been minimized.

JackHerring commented Aug 8, 2017

@AndriySvyryd will owned types be released in 2.0.0 if it is not fully implemented?

@AndriySvyryd

This comment has been minimized.

Member

AndriySvyryd commented Aug 8, 2017

@JackHerring Yes. You can map optional owned types to a different table.

@JackHerring

This comment has been minimized.

JackHerring commented Aug 9, 2017

@AndriySvyryd how do I go about this when using TPH?

@AndriySvyryd

This comment has been minimized.

Member

AndriySvyryd commented Aug 9, 2017

@JackHerring Owned types don't support inheritance, so I assume that you mean that the owner is using TPH, which doesn't affect configuration:

modelBuilder.Entity<Owner>().OwnsOne(p => p.Owned).ToTable("Owned");
@jrote1

This comment has been minimized.

jrote1 commented Sep 14, 2017

Is there any progress on the resolution of this issue?

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Sep 14, 2017

@jrote1 This issue is in the Backlog milestone. This means that it is not going to happen for the 2.1 release. We will re-assess the backlog following the 2.1 release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

@AndriySvyryd

This comment has been minimized.

Member

AndriySvyryd commented Sep 14, 2017

@jrote1 You can vote for features by using the +1 reaction (👍) on the original comment

@space-alien

This comment has been minimized.

space-alien commented Dec 8, 2017

I would really like to see this. The non-nullability of an owned type seriously restricts the usefulness of this feature.

@julielerman

This comment has been minimized.

julielerman commented Dec 8, 2017

I have a 1/2 written blog post on a great pattern to use until this is supported (something that @anpete and @divega helped me work out). It was getting long and I completely forgot to go finish it up! Will try to get to it over the weekend.

@space-alien

This comment has been minimized.

space-alien commented Dec 8, 2017

Sounds great Julie, look forward to reading it :)

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Feb 3, 2018

Note from triage: remember to add API for configuring optional owned types, whether relational/table splitting or not, when implementing this. See #10818

@AndriySvyryd

This comment has been minimized.

Member

AndriySvyryd commented Feb 13, 2018

We should also handle Added and Deleted dependents for Unchanged principals as updates in the database.
Also a way to implement this for dependents that have at least one required property is to use that property as the marker. Also @divega idea would work with this by creating a required shadow property with a client-side default value.
If all non-key properties are nullable we would return a null only if all values are null, this is inefficient, so we should warn and provide the above workaround.

@AndriySvyryd

This comment has been minimized.

Member

AndriySvyryd commented Feb 14, 2018

Attention all subscribers: Could you provide a simple description of the scenario that this feature would enable?

  1. Show sample entity types and corresponding table structure.
  2. Would you need it primarily for owned types (allows the same CLR type to be mapped to multiple entity types) or for plain table splitting (allows the dependent type to be in a hierarchy)?
  3. Currently OwnsOne maps the owned type to the same table as the principal (table splitting) by default, but you can map it to a separate table. If we introduce OwnsOneOptional would the expected default would also be to use table splitting?
@niwrA

This comment has been minimized.

niwrA commented Apr 13, 2018

Probably the most important point here would be that you want to separate the decision for how your entity is designed from your database design. A separate table or not for, say, a primary address, should be a decision that is transparent from that you want this to appear as a separate object on your entity.

In that sense, an owned type is basically a shortcut notation for a 1-to-[0..1] relation, and the fact that the owned object is rendered as null / none (f#) the most clear way of expressing that it is currently 1-to-[0], rather than 1-to-1 with no properties set, so that you might need to add properties to indicate whether it is set or not.

As I seem to recall hearing on a podcast (.NET Rocks probably) that you are investigating using Code-First EF modelling for MongoDb and other DocumentDb type databases. In those systems, a child json object will simply be or not be there in the document, and that will result in the behavior as people have expressed to want to see here. And then you want consistent behavior whether you use a sql or documentdb type datastore backing.

Similarly, you may want to optimise your database by having more data per table or less in terms of partitioning etc. This should not have any influence on your Code First modeling, preferably.

I currently always design and implement my repositories at least partly in both SQL(Lite) and MongoDb and use them for state objects that are injected into business objects that only define through interfaces what state (and repository) functionality they need, so neither layer knows about the other and there are no dependencies on either side. The best way to validate this is by being able to DI either repository implementation into the business logic and everything still works and works well. This means preferably going through the root entity (aggregate) by default and then working down from there, which suits me well because I follow Domain Driven Design principles (they also work for MicroServices).

In that sense, as I interact with injected state objects, the fact that EFCore would always give back a state object for the owned entity but MongoDb wouldn't is a specific scenario for me personally, especially as with 2.0, EFCore is starting to reach a level of quality that makes me want to use it over almost anything else out there. ;) (so good job everyone)

@ajcvickers ajcvickers added this to the 3.0.0 milestone May 17, 2018

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented May 17, 2018

Plan for 3.0: If all properties are null, then return null entity. Initially, no flag to configure this behavior and no more complex mapping cases (e.g. using required property or discriminator), but could do so based on feedback.

@AndriySvyryd

This comment has been minimized.

Member

AndriySvyryd commented May 17, 2018

And all the dependent columns will be created as nullable.

@soycabanillas

This comment has been minimized.

soycabanillas commented Jul 7, 2018

Normally returning a null entity when all properties are null will work fine, but in some other cases there will be a lost of fidelity not acceptable. I'm thinking on serialization and deserialization.

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