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

Snapshot should preserve the property CLR type when value conversion is used #12212

Closed
AndriySvyryd opened this issue Jun 2, 2018 · 1 comment · Fixed by #16484
Closed

Snapshot should preserve the property CLR type when value conversion is used #12212

AndriySvyryd opened this issue Jun 2, 2018 · 1 comment · Fixed by #16484
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone
3.0.0

Comments

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Jun 2, 2018 •
edited
Loading

If the value converted changes the CLR type and no explicit column type has been set then the snapshot should store the model CLR type as the mapping for the provider CLR type might be different.

See PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#580
@ajcvickers ajcvickers added this to the 2.2.0 milestone Jun 6, 2018
@ajcvickers ajcvickers changed the title Snapshot should store column type when value conversion is used Snapshot should preserve the property CLR type when value conversion is used Jun 6, 2018
@ajcvickers
Copy link
Member

Rather than setting the column type explicitly, which is very provider-specific, we should preserve the property CLR type even when there is a conversion without hints.

@caleblloyd caleblloyd mentioned this issue Jun 11, 2018
Closed
@ajcvickers ajcvickers modified the milestones: 2.2.0-preview2, 2.2.0 Sep 11, 2018
@ajcvickers ajcvickers modified the milestones: 2.2.0-preview3, 2.2.0, 3.0.0 Oct 15, 2018
@AndriySvyryd AndriySvyryd mentioned this issue Feb 23, 2019
Merged
@roji roji assigned roji and unassigned AndriySvyryd Feb 25, 2019
@ajcvickers ajcvickers self-assigned this Jun 16, 2019
@ajcvickers ajcvickers mentioned this issue Jul 2, 2019
Open
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed breaking-change labels Jul 6, 2019
ajcvickers added a commit that referenced this issue Jul 6, 2019
@ajcvickers
Store database type name in model snapshot
1777f04 
Fixes #16239
Fixes #12212

Also, make the SnapshotModelProcessor finalize the model.

I investigated whether it would be better to only store this sometimes, but that then still requires making some assumptions (basically using some conventions and/or the type mapper doing what it does in conventions) when loading the snapshot to ensure we find the right type for the provider being used. Storing it always is clean and simple, decouples from conventions when loading, and means we don't have to do special magic for facets that come from value converters.
@ajcvickers ajcvickers mentioned this issue Jul 6, 2019
Merged
ajcvickers added a commit that referenced this issue Jul 7, 2019
@ajcvickers
Store database type name in model snapshot
e454071 
Fixes #16239
Fixes #12212

Also, make the SnapshotModelProcessor finalize the model.

I investigated whether it would be better to only store this sometimes, but that then still requires making some assumptions (basically using some conventions and/or the type mapper doing what it does in conventions) when loading the snapshot to ensure we find the right type for the provider being used. Storing it always is clean and simple, decouples from conventions when loading, and means we don't have to do special magic for facets that come from value converters.
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview8 Jul 29, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview8, 3.0.0 Nov 11, 2019
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Milestone
3.0.0
Development

Successfully merging a pull request may close this issue.

Store database type name in model snapshot dotnet/efcore
3 participants
@ajcvickers @roji @AndriySvyryd