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.

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

[SMALL, RevEng] Map SQLServer xml columns to string properties. #4629

Closed
wants to merge 1 commit into from

Conversation

lajones
Copy link
Contributor

@lajones lajones commented Feb 23, 2016

Partial fix for #3076. Update RevEng SQL Server provider to map xml columns to string properties.

@natemcmaster
Copy link
Contributor

Have we considered the limitations of xml data type? This simple PR works from database => model. Does EF handle xml columns types the other way?

@lajones
Copy link
Contributor Author

lajones commented Feb 23, 2016

@natemcmaster Yes - I did some testing and EF handles insert, update and delete going the other way. You can get into problems if you try to store some (but not all) non-XML e.g. ">'<" causes problems but just "someText>" does not. You get a DbUpdateException which explains (in the inner exception) that it had trouble parsing the string as XML. Discussed these limitations with @rowanmiller and decided they were OK.

@rowanmiller
Copy link
Contributor

Yeah I've done some playing around with it for demos etc. and it works for the mainline cases just fine.

@natemcmaster
Copy link
Contributor

Ok. Changes here look fine then. :shipit:

@lajones
Copy link
Contributor Author

lajones commented Feb 23, 2016

Checked in with commit a8b185f.

@lajones lajones closed this Feb 23, 2016
@lajones lajones added this to the 1.0.0-rc2 milestone Feb 23, 2016
@lajones lajones self-assigned this Feb 23, 2016
@lajones lajones deleted the 160222-lajones_Issue3076_Xml_01 branch February 23, 2016 19:24
@ajcvickers ajcvickers removed this from the 1.0.0-rc2 milestone Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants