Fix rowversion null behaviour (#998)

dotnet · Apr 14, 2021 · e4d87e6 · e4d87e6
1 parent c34e8a4
commit e4d87e6
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 16 deletions.
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs
@@ -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

diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs
@@ -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

diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs
@@ -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)]
@@ -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;
+            }
+        }
     }
 }
diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs
@@ -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;
@@ -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()
         {
@@ -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");
+            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;
+        }
     }
 }