Skip to content

Conversation

@maumar
Copy link
Contributor

@maumar maumar commented Feb 10, 2024

Port of #33052
Fixes #33004

Description
When figuring out nullability of columns representing a given property, if property is declared on derived entity in TPH, we make that column nullable. For complex type properties we should be doing the same (and we did), but declaring type of that property is the complex type itself. Instead we should look at the ContainingEntityType rather than just DeclaringType.

Customer impact
For scenario where customer defines a complex type on a derived entity in TPH hierarchy and that complex type has a required property we generate incorrect model. The result it is impossible to create an entity in that hierarchy that doesn't contain the complex type (this should be allowed for base types, if the complex type is defined on derived). Workaround is to manually modify the migration and set all the necessary columns to nullable.

How found
Customer reported on 8.

Regression
No, complex type support is a new feature in EF8.

Testing
Test added.

Risk
Very Low. Fix is a one line and a very straightforward change. We add call to the API that properly takes complex types into account when figuring out declaring entity type. We use that API in numerous places already and it's a no-op for non-complex type cases. Added quirk just to be safe.

@maumar maumar changed the title Fix to #33004 - Unfulfillable nullable contraints are set for complextypes with TPH entities [release/8.0] Fix to #33004 - Unfulfillable nullable contraints are set for complextypes with TPH entities Feb 10, 2024
@AndriySvyryd
Copy link
Member

Note that the branch is currently closed

@ajcvickers ajcvickers added this to the 8.0.4 milestone Feb 15, 2024
… types with TPH entities

When figuring out nullability of columns representing a given property, if property is declared on derived entity in TPH, we make that column nullable. For complex type properties we should be doing the same (and we did), but declaring type of that property is the complex type itself. Instead we should look at the ContainingEntityType rather than just DeclaringType.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants