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.

Sign up for GitHub

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

Jump to bottom

Complete the sample with the typed discriminator mapping #4322

Merged
merged 6 commits into from
Apr 25, 2023
Merged

Complete the sample with the typed discriminator mapping #4322

merged 6 commits into from
Apr 25, 2023

Conversation

eluchsinger
Copy link
Contributor

@eluchsinger eluchsinger commented Apr 14, 2023 •
edited
Loading

The sample is missing a HasValue statement, which actually does the mapping. The HasValue statement is available for the sample with shadow mappings.

These docs will be changed by this PR: Inheritance - Table per Hierarchy And Discriminator Configuration

This other sample using untyped mappings for the discriminator is using the HasValue statement correctly: Sample with shadow mappings

@eluchsinger
Complete the sample with the typed discriminator mapping
16d1016 
The sample is missing a `HasValue` statement, which actually does the mapping. The `HasValue` statement is available for the sample with shadow-mappings.
@ajcvickers
Copy link
Contributor

@eluchsinger The call to HasValue should not be needed here. By default, EF will use the type name for string discriminator values.

@AndriySvyryd In the actual code sample, the subtype is not mapped. One way to map it would be to have HasValue calls for the subtypes, but this might imply that the HasValue calls are needed. Would it be better to just make sure that the subtype is mapped?

@AndriySvyryd
Copy link
Member

Would it be better to just make sure that the subtype is mapped?

Yep.

@ajcvickers ajcvickers merged commit af7aaf8 into dotnet:main Apr 25, 2023
@ajcvickers
Copy link
Contributor

Thanks!

@ajcvickers ajcvickers mentioned this pull request Apr 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajcvickers ajcvickers ajcvickers approved these changes

Assignees
No one assigned
Labels
community-contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eluchsinger @ajcvickers @AndriySvyryd