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.

Sign up for GitHub

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

Jump to bottom

Fix rowversion null behaviour #998

Merged
merged 3 commits into from
Apr 14, 2021
Merged

Fix rowversion null behaviour #998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d480d9
Change SqlDataReader RowVersion/Timestamp null behaviour to match oth…
Wraith2 Mar 20, 2021
222e821
add test
Wraith2 Mar 28, 2021
fe9fdb7
rework positive test and add negative test
Wraith2 Apr 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3732,16 +3732,26 @@ private bool TryReadColumnInternal(int i, bool readHeaderOnly = false)
_sharedState._nextColumnDataToRead = _sharedState._nextColumnHeaderToRead;
_sharedState._nextColumnHeaderToRead++; // We read this one

if (isNull && columnMetaData.type != SqlDbType.Timestamp)
if (isNull)
{
TdsParser.GetNullSqlValue(_data[_sharedState._nextColumnDataToRead],
columnMetaData,
_command != null ? _command.ColumnEncryptionSetting : SqlCommandColumnEncryptionSetting.UseConnectionSetting,
_parser.Connection);

if (!readHeaderOnly)
if (columnMetaData.type == SqlDbType.Timestamp)
{
_sharedState._nextColumnDataToRead++;
if (!LocalAppContextSwitches.LegacyRowVersionNullBehaviour)
{
_data[i].SetToNullOfType(SqlBuffer.StorageType.SqlBinary);
}
}
else
{
TdsParser.GetNullSqlValue(_data[_sharedState._nextColumnDataToRead],
columnMetaData,
_command != null ? _command.ColumnEncryptionSetting : SqlCommandColumnEncryptionSetting.UseConnectionSetting,
_parser.Connection);

if (!readHeaderOnly)
{
_sharedState._nextColumnDataToRead++;
}
}
}
else
Expand Down
26 changes: 18 additions & 8 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4279,16 +4279,26 @@ private bool TryReadColumnInternal(int i, bool readHeaderOnly = false)
_sharedState._nextColumnDataToRead = _sharedState._nextColumnHeaderToRead;
_sharedState._nextColumnHeaderToRead++; // We read this one

if (isNull && columnMetaData.type != SqlDbType.Timestamp /* Maintain behavior for known bug (Dev10 479607) rejected as breaking change - See comments in GetNullSqlValue for timestamp */)
if (isNull)
{
TdsParser.GetNullSqlValue(_data[_sharedState._nextColumnDataToRead],
columnMetaData,
_command != null ? _command.ColumnEncryptionSetting : SqlCommandColumnEncryptionSetting.UseConnectionSetting,
_parser.Connection);

if (!readHeaderOnly)
if (columnMetaData.type == SqlDbType.Timestamp)
{
_sharedState._nextColumnDataToRead++;
if (!LocalAppContextSwitches.LegacyRowVersionNullBehaviour)
{
_data[i].SetToNullOfType(SqlBuffer.StorageType.SqlBinary);
}
}
else
{
TdsParser.GetNullSqlValue(_data[_sharedState._nextColumnDataToRead],
columnMetaData,
_command != null ? _command.ColumnEncryptionSetting : SqlCommandColumnEncryptionSetting.UseConnectionSetting,
_parser.Connection);

if (!readHeaderOnly)
{
_sharedState._nextColumnDataToRead++;
}
}
}
else
Expand Down
26 changes: 26 additions & 0 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ namespace Microsoft.Data.SqlClient
internal static partial class LocalAppContextSwitches
{
internal const string MakeReadAsyncBlockingString = @"Switch.Microsoft.Data.SqlClient.MakeReadAsyncBlocking";
internal const string LegacyRowVersionNullString = @"Switch.Microsoft.Data.SqlClient.LegacyRowVersionNullBehaviour";

private static bool _makeReadAsyncBlocking;
private static bool? s_legacyRowVersionNullBehaviour;

public static bool MakeReadAsyncBlocking
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand All @@ -19,5 +23,27 @@ public static bool MakeReadAsyncBlocking
return AppContext.TryGetSwitch(MakeReadAsyncBlockingString, out _makeReadAsyncBlocking) ? _makeReadAsyncBlocking : false;
}
}

/// <summary>
/// In System.Data.SqlClient and Microsoft.Data.SqlClient prior to 3.0.0 a field with type Timestamp/RowVersion
/// would return an empty byte array. This switch contols whether to preserve that behaviour on newer versions
/// of Microsoft.Data.SqlClient, if this switch returns false an appropriate null value will be returned
/// </summary>
public static bool LegacyRowVersionNullBehaviour
{
get
{
if (s_legacyRowVersionNullBehaviour == null)
{
bool value = false;
if (AppContext.TryGetSwitch(LegacyRowVersionNullString, out bool providedValue))
{
value = providedValue;
}
s_legacyRowVersionNullBehaviour = value;
}
return s_legacyRowVersionNullBehaviour.Value;
}
}
}
}
76 changes: 76 additions & 0 deletions src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Data.SqlTypes;
using System.Reflection;
using System.Text;
using System.Threading;
using Xunit;
Expand All @@ -13,6 +15,8 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests
{
public static class DataReaderTest
{
private static object s_rowVersionLock = new object();

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
public static void LoadReaderIntoDataTableToTestGetSchemaTable()
{
Expand Down Expand Up @@ -288,5 +292,77 @@ first_name varchar(100) null,
// hidden field
Assert.Contains("user_id", names, StringComparer.Ordinal);
}

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
public static void CheckNullRowVersionIsBDNull()
{
lock (s_rowVersionLock)
{
bool? originalValue = SetLegacyRowVersionNullBehaviour(false);
try
{
using (SqlConnection con = new SqlConnection(DataTestUtility.TCPConnectionString))
{
con.Open();
using (SqlCommand command = con.CreateCommand())
{
command.CommandText = "select cast(null as rowversion) rv";
using (SqlDataReader reader = command.ExecuteReader())
{
reader.Read();
Assert.True(reader.IsDBNull(0));
Assert.Equal(reader[0], DBNull.Value);
}
}
}
}
finally
{
SetLegacyRowVersionNullBehaviour(originalValue);
}
}
}

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
public static void CheckLegacyNullRowVersionIsEmptyArray()
{
lock (s_rowVersionLock)
{
bool? originalValue = SetLegacyRowVersionNullBehaviour(true);
try
{
using (SqlConnection con = new SqlConnection(DataTestUtility.TCPConnectionString))
{
con.Open();
using (SqlCommand command = con.CreateCommand())
{
command.CommandText = "select cast(null as rowversion) rv";
using (SqlDataReader reader = command.ExecuteReader())
{
reader.Read();
Assert.False(reader.IsDBNull(0));
SqlBinary value = reader.GetSqlBinary(0);
Assert.False(value.IsNull);
Assert.Equal(0, value.Length);
Assert.NotNull(value.Value);
}
}
}
}
finally
{
SetLegacyRowVersionNullBehaviour(originalValue);
}
}
}

private static bool? SetLegacyRowVersionNullBehaviour(bool? value)
{
Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches");
cheenamalhotra marked this conversation as resolved.
Show resolved Hide resolved
FieldInfo switchField = switchesType.GetField("s_legacyRowVersionNullBehaviour", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static);
bool? originalValue = (bool?)switchField.GetValue(null);
switchField.SetValue(null, value);
return originalValue;
}
}
}