Skip to content

Avoid using SqlXml for serialization#38506

Open
AndriySvyryd wants to merge 3 commits into
mainfrom
FasterXML
Open

Avoid using SqlXml for serialization#38506
AndriySvyryd wants to merge 3 commits into
mainfrom
FasterXML

Conversation

@AndriySvyryd

Copy link
Copy Markdown
Member

Follow-up to #38462 improving performance

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR is a follow-up to #38462, changing SQL Server xml parameter handling to avoid wrapping XML in SqlXml (and its extra buffering) by sending an XmlReader directly to SqlClient, improving performance while keeping the SqlDbType.Xml path.

Changes:

  • Switch SqlServerStringTypeMapping to set SqlParameter.Value to an XmlReader for xml store type instead of SqlXml.
  • Update SqlServerQueryStringFactory so ToQueryString() can render xml parameters whose value is an XmlReader.
  • Adjust SQL Server tests to validate the new XmlReader parameter behavior and update related comments.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs Updates unit test expectation from SqlXml to XmlReader for xml parameters.
test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs Updates functional test comments around the xml parameter path exercised by the test.
src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs Sends xml string values as a streaming XmlReader rather than buffering via SqlXml.
src/EFCore.SqlServer/Query/Internal/SqlServerQueryStringFactory.cs Adds XmlReader handling to reconstruct a printable canonical xml literal for ToQueryString().

Comment thread src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 29, 2026 20:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comment thread test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs
Comment thread src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs
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.

2 participants