Skip to content

Modify SqlStreamingXml XmlWriter to internally use a MemoryStream#3974

Open
jimhblythe wants to merge 3 commits intodotnet:mainfrom
jimhblythe:issue-1877
Open

Modify SqlStreamingXml XmlWriter to internally use a MemoryStream#3974
jimhblythe wants to merge 3 commits intodotnet:mainfrom
jimhblythe:issue-1877

Conversation

@jimhblythe
Copy link

Modify SqlStreamingXml XmlWriter to internally use a MemoryStream instead of a StringBuilder.

Note: UTF8Encoding(false) addition in s_writerSettings is consistent with prior default used within StringWriter/StringBuilder

Issues

Fixes #1877 to be O(n)

Testing

Added 2 new Manual tests to ensure linear behavior for single large node, and secondary validation for multiple nodes
image

…tead of a StringBuilder. (dotnet#1877)

Note: UTF8Encoding(false) addition in s_writerSettings is consistent with prior default used within StringWriter/StringBuilder
@jimhblythe jimhblythe requested a review from a team as a code owner February 20, 2026 21:03
@jimhblythe
Copy link
Author

@dotnet-policy-service agree

…ple elements

Enhance comments within SqlStreamingXml
Extend Manual tests to fully cover GetChars
WriteXmlElement includes uncovered paths not accessible for SQL XML column types which normalize Whitespace, CDATA, EntityReference, XmlDeclaration, ProcessingInstruction, DocumentType, and Comment node types
@paulmedynski
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@paulmedynski paulmedynski self-assigned this Feb 27, 2026
Copy link
Contributor

@paulmedynski paulmedynski left a comment

Choose a reason for hiding this comment

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

Looks great - thanks for this optimization and expanded test coverage! Just one question.

@paulmedynski
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@paulmedynski paulmedynski added this to the 7.0.0 milestone Feb 27, 2026
@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.59%. Comparing base (a68e00f) to head (9f44ae8).
⚠️ Report is 24 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (a68e00f) and HEAD (9f44ae8). Click for more details.

HEAD has 6 uploads less than BASE
Flag BASE (a68e00f) HEAD (9f44ae8)
netfx 2 0
netcore 2 0
addons 2 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3974       +/-   ##
===========================================
- Coverage   75.22%   64.59%   -10.63%     
===========================================
  Files         266      282       +16     
  Lines       42932    66080    +23148     
===========================================
+ Hits        32294    42683    +10389     
- Misses      10638    23397    +12759     
Flag Coverage Δ
PR-SqlClient-Project 64.59% <100.00%> (?)
addons ?
netcore ?
netfx ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

O(N^2) performance when reading XML with SequentialAccess

3 participants