From 630f43d66206513a567279b2f48553824f6f6fa8 Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Mon, 18 Mar 2024 09:10:46 +1000 Subject: [PATCH 01/11] serialse scale for vartime types when it is has been explicitly set by the user to zero --- .../src/Microsoft/Data/SqlClient/SqlParameter.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs index ee3a09c17a..b4da90fe2f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs @@ -583,7 +583,7 @@ internal byte ScaleInternal } } - private bool ShouldSerializeScale() => _scale != 0; // V1.0 compat, ignore _hasScale + private bool ShouldSerializeScale() => _scale != 0 || (GetMetaTypeOnly().IsVarTime && HasFlag(SqlParameterFlags.HasScale)); /// [ @@ -1509,8 +1509,7 @@ internal byte GetActualScale() return ScaleInternal; } - // issue: how could a user specify 0 as the actual scale? - if (GetMetaTypeOnly().IsVarTime) + if (GetMetaTypeOnly().IsVarTime && !HasFlag(SqlParameterFlags.HasScale)) { return TdsEnums.DEFAULT_VARTIME_SCALE; } From 1b9eca46ae7224da85541af3677b6e61b468dd92 Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Mon, 18 Mar 2024 15:36:22 +1000 Subject: [PATCH 02/11] add context switch to retain legacy scale serialisation behaviour by default --- .../Data/SqlClient/LocalAppContextSwitches.cs | 48 +++++++++-- .../Microsoft/Data/SqlClient/SqlParameter.cs | 24 +++++- .../tests/FunctionalTests/SqlParameterTest.cs | 82 +++++++++++++++++++ 3 files changed, 144 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index ed2c4aedf7..7490df9da0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -14,17 +14,22 @@ private enum Tristate : byte False = 1, True = 2 } + private static bool IsNotInitialized(Tristate value) => value == Tristate.NotInitialized; + private static bool IsTrue(Tristate value) => value == Tristate.True; internal const string MakeReadAsyncBlockingString = @"Switch.Microsoft.Data.SqlClient.MakeReadAsyncBlocking"; internal const string LegacyRowVersionNullString = @"Switch.Microsoft.Data.SqlClient.LegacyRowVersionNullBehavior"; internal const string SuppressInsecureTLSWarningString = @"Switch.Microsoft.Data.SqlClient.SuppressInsecureTLSWarning"; internal const string UseMinimumLoginTimeoutString = @"Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin"; + internal const string LegacyVarTimeZeroScaleBehaviourString = @"Switch.Microsoft.Data.SqlClient.LegacyVarTimeZeroScaleBehaviour"; // this field is accessed through reflection in tests and should not be renamed or have the type changed without refactoring NullRow related tests private static Tristate s_legacyRowVersionNullBehavior; private static Tristate s_suppressInsecureTLSWarning; private static Tristate s_makeReadAsyncBlocking; private static Tristate s_useMinimumLoginTimeout; + // this field is accessed through reflection in Microsoft.Data.SqlClient.Tests.SqlParameterTests and should not be renamed or have the type changed without refactoring related tests + private static Tristate s_legacyVarTimeZeroScaleBehaviour; #if !NETFRAMEWORK static LocalAppContextSwitches() @@ -90,7 +95,7 @@ public static bool SuppressInsecureTLSWarning { get { - if (s_suppressInsecureTLSWarning == Tristate.NotInitialized) + if (IsNotInitialized(s_suppressInsecureTLSWarning)) { if (AppContext.TryGetSwitch(SuppressInsecureTLSWarningString, out bool returnedValue) && returnedValue) { @@ -101,7 +106,7 @@ public static bool SuppressInsecureTLSWarning s_suppressInsecureTLSWarning = Tristate.False; } } - return s_suppressInsecureTLSWarning == Tristate.True; + return IsTrue(s_suppressInsecureTLSWarning); } } @@ -115,7 +120,7 @@ public static bool LegacyRowVersionNullBehavior { get { - if (s_legacyRowVersionNullBehavior == Tristate.NotInitialized) + if (IsNotInitialized(s_legacyRowVersionNullBehavior)) { if (AppContext.TryGetSwitch(LegacyRowVersionNullString, out bool returnedValue) && returnedValue) { @@ -126,7 +131,7 @@ public static bool LegacyRowVersionNullBehavior s_legacyRowVersionNullBehavior = Tristate.False; } } - return s_legacyRowVersionNullBehavior == Tristate.True; + return IsTrue(s_legacyRowVersionNullBehavior); } } @@ -138,7 +143,7 @@ public static bool MakeReadAsyncBlocking { get { - if (s_makeReadAsyncBlocking == Tristate.NotInitialized) + if (IsNotInitialized(s_makeReadAsyncBlocking)) { if (AppContext.TryGetSwitch(MakeReadAsyncBlockingString, out bool returnedValue) && returnedValue) { @@ -149,7 +154,7 @@ public static bool MakeReadAsyncBlocking s_makeReadAsyncBlocking = Tristate.False; } } - return s_makeReadAsyncBlocking == Tristate.True; + return IsTrue(s_makeReadAsyncBlocking); } } @@ -162,7 +167,7 @@ public static bool UseMinimumLoginTimeout { get { - if (s_useMinimumLoginTimeout == Tristate.NotInitialized) + if (IsNotInitialized(s_useMinimumLoginTimeout)) { if (AppContext.TryGetSwitch(UseMinimumLoginTimeoutString, out bool returnedValue) && returnedValue) { @@ -173,7 +178,34 @@ public static bool UseMinimumLoginTimeout s_useMinimumLoginTimeout = Tristate.False; } } - return s_useMinimumLoginTimeout == Tristate.True; + return IsTrue(s_useMinimumLoginTimeout); + } + } + + + /// + /// When set to 'true' this will output a scale value of 7 (DEFAULT_VARTIME_SCALE) when the scale + /// is explicitly set to zero for VarTime data types ('datetime2', 'datetimeoffset' and 'time') + /// If no scale is set explicitly it will continue to output scale of 7 (DEFAULT_VARTIME_SCALE) + /// regardsless of switch value. + /// This app context switch defaults to 'true'. + /// + public static bool LegacyVarTimeZeroScaleBehaviour + { + get + { + if (IsNotInitialized(s_legacyVarTimeZeroScaleBehaviour)) + { + if (!AppContext.TryGetSwitch(LegacyVarTimeZeroScaleBehaviourString, out bool returnedValue) || !returnedValue) + { + s_legacyVarTimeZeroScaleBehaviour = Tristate.False; + } + else + { + s_legacyVarTimeZeroScaleBehaviour = Tristate.True; + } + } + return IsTrue(s_legacyVarTimeZeroScaleBehaviour); } } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs index b4da90fe2f..1e2eefa31b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs @@ -583,7 +583,27 @@ internal byte ScaleInternal } } - private bool ShouldSerializeScale() => _scale != 0 || (GetMetaTypeOnly().IsVarTime && HasFlag(SqlParameterFlags.HasScale)); + private bool ShouldSerializeScale() + { + if (LocalAppContextSwitches.LegacyVarTimeZeroScaleBehaviour) + { + return ShouldSerializeScale_Legacy(); + } + if (_scale != 0 || (GetMetaTypeOnly().IsVarTime && HasFlag(SqlParameterFlags.HasScale))) + { + return true; + } + return false; + } + + private bool ShouldSerializeScale_Legacy() + { + if (_scale != 0) + { + return true; + } + return false; + } /// [ @@ -1509,7 +1529,7 @@ internal byte GetActualScale() return ScaleInternal; } - if (GetMetaTypeOnly().IsVarTime && !HasFlag(SqlParameterFlags.HasScale)) + if (GetMetaTypeOnly().IsVarTime) { return TdsEnums.DEFAULT_VARTIME_SCALE; } diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs index a5172b04bf..f06024bde0 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs @@ -6,6 +6,7 @@ using System.Data; using System.Data.Common; using System.Data.SqlTypes; +using System.Reflection; using Xunit; namespace Microsoft.Data.SqlClient.Tests @@ -1851,5 +1852,86 @@ private enum Int64Enum : long A = long.MinValue, B = long.MaxValue } + + + private static readonly object _parameterLegacyScaleLock = new(); + + [Theory] + [InlineData(0, 0)] + [InlineData(1, 1)] + [InlineData(2, 2)] + [InlineData(3, 3)] + [InlineData(4, 4)] + [InlineData(5, 5)] + [InlineData(6, 6)] + [InlineData(7, 7)] + public void SqlTypes_Datetime2_Scale(byte setScale, byte outputScale) + { + lock (_parameterLegacyScaleLock) + { + var originalValue = SetLegacyVarTimeZeroScaleBehaviour(false); + try + { + var parameter = new SqlParameter + { + DbType = DbType.DateTime2, + Scale = setScale + }; + + var actualScale = (byte)typeof(SqlParameter).GetMethod("GetActualScale", BindingFlags.NonPublic | BindingFlags.Instance).Invoke(parameter, null); + + Assert.Equal(outputScale, actualScale); + } + + finally + { + SetLegacyVarTimeZeroScaleBehaviour(originalValue); + } + } + } + + [Theory] + [InlineData(0, 7)] + [InlineData(1, 1)] + [InlineData(2, 2)] + [InlineData(3, 3)] + [InlineData(4, 4)] + [InlineData(5, 5)] + [InlineData(6, 6)] + [InlineData(7, 7)] + public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) + { + lock (_parameterLegacyScaleLock) + { + var originalValue = SetLegacyVarTimeZeroScaleBehaviour(true); + try + { + var parameter = new SqlParameter + { + DbType = DbType.DateTime2, + Scale = setScale + }; + + var actualScale = (byte)typeof(SqlParameter).GetMethod("GetActualScale", BindingFlags.NonPublic | BindingFlags.Instance).Invoke(parameter, null); + + Assert.Equal(outputScale, actualScale); + } + + finally + { + SetLegacyVarTimeZeroScaleBehaviour(originalValue); + } + } + } + + private static bool? SetLegacyVarTimeZeroScaleBehaviour(bool? value) + { + var switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); + var switchField = switchesType.GetField("s_legacyVarTimeZeroScaleBehaviour", BindingFlags.NonPublic | BindingFlags.Static); + var originalValue = (byte)switchField.GetValue(null); + byte triStateValue = value.HasValue ? value.Value ? (byte)2 : (byte)1 : (byte)0; + switchField.SetValue(null, triStateValue); + return originalValue == 0 ? null : originalValue == 2; + } } } From 9d0bd8a0534970f1e9066688180b5d69b3766873 Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Tue, 19 Mar 2024 09:03:06 +1000 Subject: [PATCH 03/11] Correct the default value of LegacyVarTimeZeroScaleBehaviour --- .../Data/SqlClient/LocalAppContextSwitches.cs | 6 +++--- .../tests/FunctionalTests/SqlParameterTest.cs | 12 +++++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index 7490df9da0..a5bb2ba70a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -196,13 +196,13 @@ public static bool LegacyVarTimeZeroScaleBehaviour { if (IsNotInitialized(s_legacyVarTimeZeroScaleBehaviour)) { - if (!AppContext.TryGetSwitch(LegacyVarTimeZeroScaleBehaviourString, out bool returnedValue) || !returnedValue) + if (!AppContext.TryGetSwitch(LegacyVarTimeZeroScaleBehaviourString, out bool returnedValue)) { - s_legacyVarTimeZeroScaleBehaviour = Tristate.False; + s_legacyVarTimeZeroScaleBehaviour = Tristate.True; } else { - s_legacyVarTimeZeroScaleBehaviour = Tristate.True; + s_legacyVarTimeZeroScaleBehaviour = returnedValue ? Tristate.False : Tristate.True; } } return IsTrue(s_legacyVarTimeZeroScaleBehaviour); diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs index f06024bde0..a15d2f7cee 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs @@ -1924,6 +1924,16 @@ public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) } } + [Fact] + public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True() + { + var switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); + + var legacyVarTimeZeroScaleBehaviour = (bool)switchesType.GetProperty("LegacyVarTimeZeroScaleBehaviour", BindingFlags.Public | BindingFlags.Static).GetValue(null); + + Assert.True(legacyVarTimeZeroScaleBehaviour); + } + private static bool? SetLegacyVarTimeZeroScaleBehaviour(bool? value) { var switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); @@ -1932,6 +1942,6 @@ public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) byte triStateValue = value.HasValue ? value.Value ? (byte)2 : (byte)1 : (byte)0; switchField.SetValue(null, triStateValue); return originalValue == 0 ? null : originalValue == 2; - } + } } } From 2463ee1d865d5c75170aff3b65180c2997b60fd3 Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Mon, 8 Apr 2024 09:41:31 +1000 Subject: [PATCH 04/11] undo refactor to make code review simpler --- .../Data/SqlClient/LocalAppContextSwitches.cs | 22 +++++++++---------- .../Microsoft/Data/SqlClient/SqlParameter.cs | 10 ++------- .../tests/FunctionalTests/SqlParameterTest.cs | 6 ++--- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index a5bb2ba70a..69e1c96e41 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -14,8 +14,6 @@ private enum Tristate : byte False = 1, True = 2 } - private static bool IsNotInitialized(Tristate value) => value == Tristate.NotInitialized; - private static bool IsTrue(Tristate value) => value == Tristate.True; internal const string MakeReadAsyncBlockingString = @"Switch.Microsoft.Data.SqlClient.MakeReadAsyncBlocking"; internal const string LegacyRowVersionNullString = @"Switch.Microsoft.Data.SqlClient.LegacyRowVersionNullBehavior"; @@ -95,7 +93,7 @@ public static bool SuppressInsecureTLSWarning { get { - if (IsNotInitialized(s_suppressInsecureTLSWarning)) + if (s_suppressInsecureTLSWarning == Tristate.NotInitialized) { if (AppContext.TryGetSwitch(SuppressInsecureTLSWarningString, out bool returnedValue) && returnedValue) { @@ -106,7 +104,7 @@ public static bool SuppressInsecureTLSWarning s_suppressInsecureTLSWarning = Tristate.False; } } - return IsTrue(s_suppressInsecureTLSWarning); + return s_suppressInsecureTLSWarning == Tristate.True; } } @@ -120,7 +118,7 @@ public static bool LegacyRowVersionNullBehavior { get { - if (IsNotInitialized(s_legacyRowVersionNullBehavior)) + if (s_legacyRowVersionNullBehavior == Tristate.NotInitialized) { if (AppContext.TryGetSwitch(LegacyRowVersionNullString, out bool returnedValue) && returnedValue) { @@ -131,7 +129,7 @@ public static bool LegacyRowVersionNullBehavior s_legacyRowVersionNullBehavior = Tristate.False; } } - return IsTrue(s_legacyRowVersionNullBehavior); + return s_legacyRowVersionNullBehavior== Tristate.True; } } @@ -143,7 +141,7 @@ public static bool MakeReadAsyncBlocking { get { - if (IsNotInitialized(s_makeReadAsyncBlocking)) + if (s_makeReadAsyncBlocking == Tristate.NotInitialized) { if (AppContext.TryGetSwitch(MakeReadAsyncBlockingString, out bool returnedValue) && returnedValue) { @@ -154,7 +152,7 @@ public static bool MakeReadAsyncBlocking s_makeReadAsyncBlocking = Tristate.False; } } - return IsTrue(s_makeReadAsyncBlocking); + return s_makeReadAsyncBlocking== Tristate.True; } } @@ -167,7 +165,7 @@ public static bool UseMinimumLoginTimeout { get { - if (IsNotInitialized(s_useMinimumLoginTimeout)) + if (s_useMinimumLoginTimeout == Tristate.NotInitialized) { if (AppContext.TryGetSwitch(UseMinimumLoginTimeoutString, out bool returnedValue) && returnedValue) { @@ -178,7 +176,7 @@ public static bool UseMinimumLoginTimeout s_useMinimumLoginTimeout = Tristate.False; } } - return IsTrue(s_useMinimumLoginTimeout); + return s_useMinimumLoginTimeout== Tristate.True; } } @@ -194,7 +192,7 @@ public static bool LegacyVarTimeZeroScaleBehaviour { get { - if (IsNotInitialized(s_legacyVarTimeZeroScaleBehaviour)) + if (s_legacyVarTimeZeroScaleBehaviour == Tristate.NotInitialized) { if (!AppContext.TryGetSwitch(LegacyVarTimeZeroScaleBehaviourString, out bool returnedValue)) { @@ -205,7 +203,7 @@ public static bool LegacyVarTimeZeroScaleBehaviour s_legacyVarTimeZeroScaleBehaviour = returnedValue ? Tristate.False : Tristate.True; } } - return IsTrue(s_legacyVarTimeZeroScaleBehaviour); + return s_legacyVarTimeZeroScaleBehaviour== Tristate.True; } } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs index 1e2eefa31b..c4c8d17995 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs @@ -583,6 +583,8 @@ internal byte ScaleInternal } } + private bool ShouldSerializeScale_Legacy() => _scale != 0; // V1.0 compat, ignore _hasScale + private bool ShouldSerializeScale() { if (LocalAppContextSwitches.LegacyVarTimeZeroScaleBehaviour) @@ -596,14 +598,6 @@ private bool ShouldSerializeScale() return false; } - private bool ShouldSerializeScale_Legacy() - { - if (_scale != 0) - { - return true; - } - return false; - } /// [ diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs index a15d2f7cee..4d38e17a2d 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs @@ -1927,7 +1927,7 @@ public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) [Fact] public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True() { - var switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); + Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); var legacyVarTimeZeroScaleBehaviour = (bool)switchesType.GetProperty("LegacyVarTimeZeroScaleBehaviour", BindingFlags.Public | BindingFlags.Static).GetValue(null); @@ -1936,8 +1936,8 @@ public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True() private static bool? SetLegacyVarTimeZeroScaleBehaviour(bool? value) { - var switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); - var switchField = switchesType.GetField("s_legacyVarTimeZeroScaleBehaviour", BindingFlags.NonPublic | BindingFlags.Static); + Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); + FieldInfo switchField = switchesType.GetField("s_legacyVarTimeZeroScaleBehaviour", BindingFlags.NonPublic | BindingFlags.Static); var originalValue = (byte)switchField.GetValue(null); byte triStateValue = value.HasValue ? value.Value ? (byte)2 : (byte)1 : (byte)0; switchField.SetValue(null, triStateValue); From b3529217276addffde126c8c2b1a41fbe4aa08c2 Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Mon, 8 Apr 2024 09:43:03 +1000 Subject: [PATCH 05/11] fix formatting --- .../Microsoft/Data/SqlClient/LocalAppContextSwitches.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index 69e1c96e41..a7127b3a5b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -129,7 +129,7 @@ public static bool LegacyRowVersionNullBehavior s_legacyRowVersionNullBehavior = Tristate.False; } } - return s_legacyRowVersionNullBehavior== Tristate.True; + return s_legacyRowVersionNullBehavior == Tristate.True; } } @@ -152,7 +152,7 @@ public static bool MakeReadAsyncBlocking s_makeReadAsyncBlocking = Tristate.False; } } - return s_makeReadAsyncBlocking== Tristate.True; + return s_makeReadAsyncBlocking == Tristate.True; } } @@ -176,7 +176,7 @@ public static bool UseMinimumLoginTimeout s_useMinimumLoginTimeout = Tristate.False; } } - return s_useMinimumLoginTimeout== Tristate.True; + return s_useMinimumLoginTimeout == Tristate.True; } } @@ -203,7 +203,7 @@ public static bool LegacyVarTimeZeroScaleBehaviour s_legacyVarTimeZeroScaleBehaviour = returnedValue ? Tristate.False : Tristate.True; } } - return s_legacyVarTimeZeroScaleBehaviour== Tristate.True; + return s_legacyVarTimeZeroScaleBehaviour == Tristate.True; } } } From bb78da8d6eb40c19de3838ce0837d3fe99e913fb Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Wed, 17 Apr 2024 13:10:56 +1000 Subject: [PATCH 06/11] touch file to trigger build --- .../tests/FunctionalTests/SqlParameterTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs index 4d38e17a2d..42b9aba09f 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs @@ -1928,7 +1928,6 @@ public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True() { Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); - var legacyVarTimeZeroScaleBehaviour = (bool)switchesType.GetProperty("LegacyVarTimeZeroScaleBehaviour", BindingFlags.Public | BindingFlags.Static).GetValue(null); Assert.True(legacyVarTimeZeroScaleBehaviour); From bc9017e8471e9e5d8e3879e912956ece56cc0935 Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Wed, 17 Apr 2024 19:54:57 +1000 Subject: [PATCH 07/11] touch file to trigger fragile CI build --- .../tests/FunctionalTests/SqlParameterTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs index 42b9aba09f..9d9b4b7e7f 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs @@ -1928,6 +1928,7 @@ public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True() { Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); + var legacyVarTimeZeroScaleBehaviour = (bool)switchesType.GetProperty("LegacyVarTimeZeroScaleBehaviour", BindingFlags.Public | BindingFlags.Static).GetValue(null); Assert.True(legacyVarTimeZeroScaleBehaviour); @@ -1941,6 +1942,6 @@ public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True() byte triStateValue = value.HasValue ? value.Value ? (byte)2 : (byte)1 : (byte)0; switchField.SetValue(null, triStateValue); return originalValue == 0 ? null : originalValue == 2; - } + } } } From 81c2cfce41555bf2c0b8e99bbe12c265bd2fe9bd Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Mon, 22 Apr 2024 16:03:22 +1000 Subject: [PATCH 08/11] address incomplete test coverage and fix bug found as a result --- .../Data/SqlClient/LocalAppContextSwitches.cs | 2 +- .../tests/FunctionalTests/SqlParameterTest.cs | 117 ++++++++++-------- 2 files changed, 63 insertions(+), 56 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index a7127b3a5b..bc2a302464 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -200,7 +200,7 @@ public static bool LegacyVarTimeZeroScaleBehaviour } else { - s_legacyVarTimeZeroScaleBehaviour = returnedValue ? Tristate.False : Tristate.True; + s_legacyVarTimeZeroScaleBehaviour = returnedValue ? Tristate.True : Tristate.False; } } return s_legacyVarTimeZeroScaleBehaviour == Tristate.True; diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs index 9d9b4b7e7f..d373770159 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs @@ -1857,60 +1857,48 @@ private enum Int64Enum : long private static readonly object _parameterLegacyScaleLock = new(); [Theory] - [InlineData(0, 0)] - [InlineData(1, 1)] - [InlineData(2, 2)] - [InlineData(3, 3)] - [InlineData(4, 4)] - [InlineData(5, 5)] - [InlineData(6, 6)] - [InlineData(7, 7)] - public void SqlTypes_Datetime2_Scale(byte setScale, byte outputScale) + [InlineData(null, 7, true)] + [InlineData(0, 7, true)] + [InlineData(1, 1, true)] + [InlineData(2, 2, true)] + [InlineData(3, 3, true)] + [InlineData(4, 4, true)] + [InlineData(5, 5, true)] + [InlineData(6, 6, true)] + [InlineData(7, 7, true)] + [InlineData(null, 7, false)] + [InlineData(0, 0, false)] + [InlineData(1, 1, false)] + [InlineData(2, 2, false)] + [InlineData(3, 3, false)] + [InlineData(4, 4, false)] + [InlineData(5, 5, false)] + [InlineData(6, 6, false)] + [InlineData(7, 7, false)] + [InlineData(null, 7, null)] + [InlineData(0, 7, null)] + [InlineData(1, 1, null)] + [InlineData(2, 2, null)] + [InlineData(3, 3, null)] + [InlineData(4, 4, null)] + [InlineData(5, 5, null)] + [InlineData(6, 6, null)] + [InlineData(7, 7, null)] + public void SqlDatetime2Scale_Legacy(int? setScale, byte outputScale, bool? legacyVarTimeZeroScaleSwitchValue) { lock (_parameterLegacyScaleLock) { - var originalValue = SetLegacyVarTimeZeroScaleBehaviour(false); + var originalLegacyVarTimeZeroScaleSwitchValue = SetLegacyVarTimeZeroScaleBehaviour(legacyVarTimeZeroScaleSwitchValue); try { var parameter = new SqlParameter { - DbType = DbType.DateTime2, - Scale = setScale + DbType = DbType.DateTime2 }; - - var actualScale = (byte)typeof(SqlParameter).GetMethod("GetActualScale", BindingFlags.NonPublic | BindingFlags.Instance).Invoke(parameter, null); - - Assert.Equal(outputScale, actualScale); - } - - finally - { - SetLegacyVarTimeZeroScaleBehaviour(originalValue); - } - } - } - - [Theory] - [InlineData(0, 7)] - [InlineData(1, 1)] - [InlineData(2, 2)] - [InlineData(3, 3)] - [InlineData(4, 4)] - [InlineData(5, 5)] - [InlineData(6, 6)] - [InlineData(7, 7)] - public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) - { - lock (_parameterLegacyScaleLock) - { - var originalValue = SetLegacyVarTimeZeroScaleBehaviour(true); - try - { - var parameter = new SqlParameter + if (setScale.HasValue) { - DbType = DbType.DateTime2, - Scale = setScale - }; + parameter.Scale = (byte)setScale.Value; + } var actualScale = (byte)typeof(SqlParameter).GetMethod("GetActualScale", BindingFlags.NonPublic | BindingFlags.Instance).Invoke(parameter, null); @@ -1919,7 +1907,7 @@ public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) finally { - SetLegacyVarTimeZeroScaleBehaviour(originalValue); + SetLegacyVarTimeZeroScaleBehaviour(originalLegacyVarTimeZeroScaleSwitchValue); } } } @@ -1927,21 +1915,40 @@ public void SqlDatetime2Scale_Legacy(byte setScale, byte outputScale) [Fact] public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True() { - Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); - - var legacyVarTimeZeroScaleBehaviour = (bool)switchesType.GetProperty("LegacyVarTimeZeroScaleBehaviour", BindingFlags.Public | BindingFlags.Static).GetValue(null); + var legacyVarTimeZeroScaleBehaviour = (bool)LocalAppContextSwitchesType.GetProperty("LegacyVarTimeZeroScaleBehaviour", BindingFlags.Public | BindingFlags.Static).GetValue(null); Assert.True(legacyVarTimeZeroScaleBehaviour); } + private static Type LocalAppContextSwitchesType => typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); + private static bool? SetLegacyVarTimeZeroScaleBehaviour(bool? value) { - Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); - FieldInfo switchField = switchesType.GetField("s_legacyVarTimeZeroScaleBehaviour", BindingFlags.NonPublic | BindingFlags.Static); - var originalValue = (byte)switchField.GetValue(null); - byte triStateValue = value.HasValue ? value.Value ? (byte)2 : (byte)1 : (byte)0; - switchField.SetValue(null, triStateValue); - return originalValue == 0 ? null : originalValue == 2; + const string LegacyVarTimeZeroScaleBehaviourSwitchname = @"Switch.Microsoft.Data.SqlClient.LegacyVarTimeZeroScaleBehaviour"; + + //reset internal state to "NotInitialized" so we pick up the value via AppContext + FieldInfo switchField = LocalAppContextSwitchesType.GetField("s_legacyVarTimeZeroScaleBehaviour", BindingFlags.NonPublic | BindingFlags.Static); + switchField.SetValue(null, (byte)0); + + bool? returnValue = null; + if (AppContext.TryGetSwitch(LegacyVarTimeZeroScaleBehaviourSwitchname, out var originalValue)) + { + returnValue = originalValue; + } + + if (value.HasValue) + { + AppContext.SetSwitch(LegacyVarTimeZeroScaleBehaviourSwitchname, value.Value); + } + else + { + //need to remove the switch value via reflection as AppContext does not expose a means to do that. + var switches = typeof(AppContext).GetField("s_switchMap", BindingFlags.NonPublic | BindingFlags.Static).GetValue(null); + MethodInfo removeMethod = switches.GetType().GetMethod("Remove", BindingFlags.Public | BindingFlags.Instance); + removeMethod.Invoke(switches, new[] { LegacyVarTimeZeroScaleBehaviourSwitchname }); + } + + return returnValue; } } } From 1e5eac6a8df3448664e262e0716ad23e39c4a328 Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Tue, 23 Apr 2024 13:39:52 +1000 Subject: [PATCH 09/11] fix reflection in test to work across target frameworks --- .../tests/FunctionalTests/SqlParameterTest.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs index d373770159..e38d531fd8 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs @@ -6,6 +6,7 @@ using System.Data; using System.Data.Common; using System.Data.SqlTypes; +using System.Linq; using System.Reflection; using Xunit; @@ -1943,9 +1944,18 @@ public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True() else { //need to remove the switch value via reflection as AppContext does not expose a means to do that. +#if NET5_0_OR_GREATER + var switches = typeof(AppContext).GetField("s_switches", BindingFlags.NonPublic | BindingFlags.Static).GetValue(null); + if (switches is not null) //may be null if not initialised yet + { + MethodInfo removeMethod = switches.GetType().GetMethod("Remove", BindingFlags.Public | BindingFlags.Instance, new Type[] { typeof(string) }); + removeMethod.Invoke(switches, new[] { LegacyVarTimeZeroScaleBehaviourSwitchname }); + } +#else var switches = typeof(AppContext).GetField("s_switchMap", BindingFlags.NonPublic | BindingFlags.Static).GetValue(null); MethodInfo removeMethod = switches.GetType().GetMethod("Remove", BindingFlags.Public | BindingFlags.Instance); removeMethod.Invoke(switches, new[] { LegacyVarTimeZeroScaleBehaviourSwitchname }); +#endif } return returnValue; From a775fdfac802a2a191c264102a1d28767b244ccb Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Tue, 23 Apr 2024 13:40:16 +1000 Subject: [PATCH 10/11] remove unused 'using' --- .../tests/FunctionalTests/SqlParameterTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs index e38d531fd8..5b1a2d7687 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs @@ -6,7 +6,6 @@ using System.Data; using System.Data.Common; using System.Data.SqlTypes; -using System.Linq; using System.Reflection; using Xunit; From 7006d247ec822f0ee728e793afdcb030783b6726 Mon Sep 17 00:00:00 2001 From: Eamon Hetherton Date: Wed, 9 Oct 2024 10:54:24 +1000 Subject: [PATCH 11/11] Update src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs Co-authored-by: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> --- .../src/Microsoft/Data/SqlClient/SqlParameter.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs index c4c8d17995..8f0e54258d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs @@ -591,11 +591,7 @@ private bool ShouldSerializeScale() { return ShouldSerializeScale_Legacy(); } - if (_scale != 0 || (GetMetaTypeOnly().IsVarTime && HasFlag(SqlParameterFlags.HasScale))) - { - return true; - } - return false; + return _scale != 0 || (GetMetaTypeOnly().IsVarTime && HasFlag(SqlParameterFlags.HasScale)); }