Modify SqlStreamingXml XmlWriter to internally use a MemoryStream

[jimhblythe]

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

[jimhblythe]

@dotnet-policy-service agree

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[paulmedynski]

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

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[codecov]

Codecov Report

❌ Patch coverage is 97.57576% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.82%. Comparing base (a68e00f) to head (9e62ba5).
⚠️ Report is 65 commits behind head on main.

Files with missing lines	Patch %	Lines
...qlClient/src/Microsoft/Data/SqlClient/SqlStream.cs	97.57%	4 Missing ⚠️

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

HEAD has 6 uploads less than BASE

Flag	BASE (a68e00f)	HEAD (9e62ba5)
netfx	2	0
netcore	2	0
addons	2	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3974      +/-   ##
==========================================
- Coverage   75.22%   65.82%   -9.40%     
==========================================
  Files         266      275       +9     
  Lines       42932    65896   +22964     
==========================================
+ Hits        32294    43379   +11085     
- Misses      10638    22517   +11879     

Flag	Coverage Δ
PR-SqlClient-Project	65.82% <97.57%> (?)
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.

[mdaigle]

@jimhblythe can you take a look at the tests in #4005 and #4008? They seem to suggest that this introduces a regression for non-ascii characters. You can check the pipeline results to see the test outcomes. I'd like to see some tests like that included in this PR as well, please 🙏

[jimhblythe]

@jimhblythe can you take a look at the tests in #4005 and #4008? They seem to suggest that this introduces a regression for non-ascii characters. You can check the pipeline results to see the test outcomes. I'd like to see some tests like that included in this PR as well, please 🙏

I will additionally including a fourth test to verify surrogate pairs:

        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
        public static void GetChars_SurrogatePairContent()
        {
            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
            // Surrogate Pair characters: 4 bytes each in UTF-8
            string xml = "<data>\U0001F600\U0001F525\U0001F680</data>";

Actual: "<data>😀🔥🚀</data>"

The test shows these worked with prior code.

[jimhblythe]

I had to use an alternate as MemoryStream was not the right choice. I actually optimized further with bypass of XmlWriter which used an additional buffer; more memory spared with latest commit.

Significantly more tests added :

Is exact replication of prior logic desired or more consistent with how SSMS would show an XML column?
Prior logic would occasionally convert < back to < - e.g. within CData
Prior logic would also expand \n to \r\n

Coverage complete except for null reference bullet-proofing that cannot be replicated through SqlReader which ensures values are provide:

[jimhblythe]

Regarding this from my last comment: Is exact replication of prior logic desired or more consistent with how SSMS would show an XML column?

I am reworking the logic and tests cases such that GetChars responds the same when streaming an Xml typed column as it does when reading Convert(NVarChar(max), xmlColumn) - each test case will verify reading as both data types.

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).