Model building changes from API review #36457
Conversation
roji
left a comment
roji
left a comment
There was a problem hiding this comment.
LGTM, see some comments/thoughts but nothing blocking.
src/EFCore.Relational/Extensions/RelationalComplexCollectionBuilderExtensions.cs
Show resolved
Hide resolved
| /// any release. You should only use it directly in your code with extreme caution and knowing that | ||
| /// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
| /// </remarks> | ||
| IRuntimeTypeBase ContainingEntryType |
There was a problem hiding this comment.
A general question, less about this and more about ContainingEntityType (and possibly other similar things): should these be returning null rather than this for when there is no containing thing? For example, Type.BaseType returns null for a .NET hierarchy root (and I think so do we), so return this to mean "there's no container" is a bit odd...
There was a problem hiding this comment.
Think of this more in terms of sets than inheritance. In our model everything that's tracked needs to be rooted on an entity type.
There was a problem hiding this comment.
Sure, but still seems quite odd that an entity "contains" itself...
Part of #35722
HasDefaultFullTextLanguageand related methods from entity type to modelvalueparameter inIsFullTextIndexandSetIsFullTextIndextofullTextIndex, consider making it non-nullablebool vectorIndextoIsVectorIndexToJsonGenerateDefaultConstraintNametoGetDefaultDefaultConstraintNameContainingTypetoContainingEntryTypeand move it toIRuntimeTypeBaseComplexCollectionTypePropertyBuilderIConventionTypeBaseBuilder.IndexerComplexPropertycc @artl93