From 9a70c8153416935e4b91689e1b9e2dad2b9583bb Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sun, 4 May 2025 02:22:18 +0100 Subject: [PATCH 1/4] refine logic around switching from replay to continue for SqlCachedBuffer used in xml reads --- .../src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs index 1f1670b97f..0a31740a23 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs @@ -45,9 +45,9 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser if (isAvailable) { cachedBytes = stateObj.TryTakeSnapshotStorage() as List; - if (cachedBytes != null && !isStarting && !isContinuing) + if (isStarting) { - stateObj.SetSnapshotStorage(null); + cachedBytes = null; } } @@ -64,7 +64,6 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser return result; } - // For now we only handle Plp data from the parser directly. Debug.Assert(metadata.metaType.IsPlp, "SqlCachedBuffer call on a non-plp data"); do @@ -105,10 +104,7 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser if (returnAfterAdd) { - if (isStarting || isContinuing) - { - stateObj.SetSnapshotStorage(cachedBytes); - } + stateObj.SetSnapshotStorage(cachedBytes); return result; } From 3213a403c50816b2a64dfd85c8206c8a9f62c7ed Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sun, 4 May 2025 16:39:17 +0100 Subject: [PATCH 2/4] add test and cleanup unused variable --- .../Data/SqlClient/SqlCachedBuffer.cs | 3 +- .../SQL/DataReaderTest/DataReaderTest.cs | 191 ++++++++++++++++++ 2 files changed, 192 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs index 0a31740a23..d7fdf3fa48 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs @@ -39,7 +39,7 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser { buffer = null; - (bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses(); + (bool isAvailable, bool isStarting, _) = stateObj.GetSnapshotStatuses(); List cachedBytes = null; if (isAvailable) @@ -56,7 +56,6 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser cachedBytes = new List(); } - // the very first length is already read. TdsOperationStatus result = parser.TryPlpBytesLeft(stateObj, out ulong plplength); if (result != TdsOperationStatus.Done) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index 8a04ba7e66..8cb7e15162 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -400,6 +400,197 @@ public static async Task CanReadEmployeesTableCompletelyWithNullImageAsync( return counter; } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task CanReadXmlData() + { + const string xml = @" + + Gambardella, Matthew + XML Developer's Guide + Computer + 44.95 + 2000-10-01 + An in-depth look at creating applications + with XML. + + + Ralls, Kim + Midnight Rain + Fantasy + 5.95 + 2000-12-16 + A former architect battles corporate zombies, + an evil sorceress, and her own childhood to become queen + of the world. + + + Corets, Eva + Maeve Ascendant + Fantasy + 5.95 + 2000-11-17 + After the collapse of a nanotechnology + society in England, the young survivors lay the + foundation for a new society. + + + Corets, Eva + Oberon's Legacy + Fantasy + 5.95 + 2001-03-10 + In post-apocalypse England, the mysterious + agent known only as Oberon helps to create a new life + for the inhabitants of London. Sequel to Maeve + Ascendant. + + + Corets, Eva + The Sundered Grail + Fantasy + 5.95 + 2001-09-10 + The two daughters of Maeve, half-sisters, + battle one another for control of England. Sequel to + Oberon's Legacy. + + + Randall, Cynthia + Lover Birds + Romance + 4.95 + 2000-09-02 + When Carla meets Paul at an ornithology + conference, tempers fly as feathers get ruffled. + + + Thurman, Paula + Splish Splash + Romance + 4.95 + 2000-11-02 + A deep sea diver finds true love twenty + thousand leagues beneath the sea. + + + Knorr, Stefan + Creepy Crawlies + Horror + 4.95 + 2000-12-06 + An anthology of horror stories about roaches, + centipedes, scorpions and other insects. + + + Kress, Peter + Paradox Lost + Science Fiction + 6.95 + 2000-11-02 + After an inadvertant trip through a Heisenberg + Uncertainty Device, James Salway discovers the problems + of being quantum. + + + O'Brien, Tim + Microsoft .NET: The Programming Bible + Computer + 36.95 + 2000-12-09 + Microsoft's .NET initiative is explored in + detail in this deep programmer's reference. + + + O'Brien, Tim + MSXML3: A Comprehensive Guide + Computer + 36.95 + 2000-12-01 + The Microsoft MSXML3 parser is covered in + detail, with attention to XML DOM interfaces, XSLT processing, + SAX and more. + + + Galos, Mike + Visual Studio 7: A Comprehensive Guide + Computer + 49.95 + 2001-04-16 + Microsoft Visual Studio 7 is explored in depth, + looking at how Visual Basic, Visual C++, C#, and ASP+ are + integrated into a comprehensive development + environment. + +"; + + string tableName = DataTestUtility.GenerateObjectName(); + + using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString)) + { + await connection.OpenAsync(); + + try + { + // setup + using (var dropCommand = connection.CreateCommand()) + { + dropCommand.CommandText = $"DROP TABLE IF EXISTS [{tableName}]"; + dropCommand.ExecuteNonQuery(); + } + + using (var createCommand = connection.CreateCommand()) + { + createCommand.CommandText = $"CREATE TABLE [{tableName}] (Id int PRIMARY KEY, Data xml NOT NULL)"; + createCommand.ExecuteNonQuery(); + } + + for (var i = 0; i < 1000; i++) + { + using (var insertCommand = connection.CreateCommand()) + { + insertCommand.CommandText = $"INSERT INTO [{tableName}] (Id, Data) VALUES (@id, @data)"; + insertCommand.Parameters.AddWithValue("@id", i); + insertCommand.Parameters.AddWithValue("@data", xml); + insertCommand.ExecuteNonQuery(); + } + } + + // execute + int id = 0; + + using (var command = connection.CreateCommand()) + { + command.CommandText = $"SELECT Data FROM [{tableName}] ORDER BY Id"; + using (var reader = await command.ExecuteReaderAsync()) + { + while (await reader.ReadAsync()) + { + var result = reader.GetString(0); + Assert.Equal(xml, result); + id++; + } + } + } + return id; + + } + finally + { + try + { + using (var dropCommand = connection.CreateCommand()) + { + dropCommand.CommandText = $"DROP TABLE IF EXISTS [{tableName}]"; + dropCommand.ExecuteNonQuery(); + } + } + catch + { + } + } + } + } + // Synapse: Cannot find data type 'rowversion'. [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] public static void CheckLegacyNullRowVersionIsEmptyArray() From 285fae4c425d9f98b7192afa3637a786b5187b01 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Mon, 5 May 2025 11:00:12 +0100 Subject: [PATCH 3/4] account for xml read canonicalization in tests --- .../ManualTests/SQL/DataReaderTest/DataReaderTest.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index 8cb7e15162..d1c77ca462 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -563,10 +563,18 @@ integrated into a comprehensive development command.CommandText = $"SELECT Data FROM [{tableName}] ORDER BY Id"; using (var reader = await command.ExecuteReaderAsync()) { + string expectedResult = null; while (await reader.ReadAsync()) { var result = reader.GetString(0); - Assert.Equal(xml, result); + if (expectedResult == null) + { + expectedResult = result; + } + else + { + Assert.Equal(expectedResult, result); + } id++; } } From a42d2c0b5fe8d0be2d77dc1d9964e8beeaa46283 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Mon, 5 May 2025 15:38:30 +0100 Subject: [PATCH 4/4] reduce iterations on new test --- .../ManualTests/SQL/DataReaderTest/DataReaderTest.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index d1c77ca462..e64d0fc362 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -525,7 +525,11 @@ integrated into a comprehensive development string tableName = DataTestUtility.GenerateObjectName(); - using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString)) + SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); + builder.PersistSecurityInfo = true; + builder.PacketSize = 3096; // should reproduce failure that this tests for in <100 reads + + using (var connection = new SqlConnection(builder.ToString())) { await connection.OpenAsync(); @@ -544,7 +548,7 @@ integrated into a comprehensive development createCommand.ExecuteNonQuery(); } - for (var i = 0; i < 1000; i++) + for (var i = 0; i < 100; i++) { using (var insertCommand = connection.CreateCommand()) {