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.

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

Serialise scale for vartime SqlParameter types that have been explicitly set by the user to zero #2411

Merged
merged 17 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@ private enum Tristate : byte
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()
Expand Down Expand Up @@ -176,5 +179,32 @@ public static bool UseMinimumLoginTimeout
return s_useMinimumLoginTimeout == Tristate.True;
}
}


/// <summary>
/// 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'.
/// </summary>
public static bool LegacyVarTimeZeroScaleBehaviour
{
get
{
if (s_legacyVarTimeZeroScaleBehaviour == Tristate.NotInitialized)
{
if (!AppContext.TryGetSwitch(LegacyVarTimeZeroScaleBehaviourString, out bool returnedValue))
{
s_legacyVarTimeZeroScaleBehaviour = Tristate.True;
}
else
{
s_legacyVarTimeZeroScaleBehaviour = returnedValue ? Tristate.True : Tristate.False;
}
}
return s_legacyVarTimeZeroScaleBehaviour == Tristate.True;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,17 @@ internal byte ScaleInternal
}
}

private bool ShouldSerializeScale() => _scale != 0; // V1.0 compat, ignore _hasScale
private bool ShouldSerializeScale_Legacy() => _scale != 0; // V1.0 compat, ignore _hasScale

private bool ShouldSerializeScale()
{
if (LocalAppContextSwitches.LegacyVarTimeZeroScaleBehaviour)
{
return ShouldSerializeScale_Legacy();
}
return _scale != 0 || (GetMetaTypeOnly().IsVarTime && HasFlag(SqlParameterFlags.HasScale));
}


/// <include file='../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlParameter.xml' path='docs/members[@name="SqlParameter"]/SqlDbType/*' />
[
Expand Down Expand Up @@ -1509,7 +1519,6 @@ internal byte GetActualScale()
return ScaleInternal;
}

// issue: how could a user specify 0 as the actual scale?
if (GetMetaTypeOnly().IsVarTime)
{
return TdsEnums.DEFAULT_VARTIME_SCALE;
Expand Down
108 changes: 108 additions & 0 deletions src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1851,5 +1852,112 @@ private enum Int64Enum : long
A = long.MinValue,
B = long.MaxValue
}


private static readonly object _parameterLegacyScaleLock = new();

[Theory]
[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 originalLegacyVarTimeZeroScaleSwitchValue = SetLegacyVarTimeZeroScaleBehaviour(legacyVarTimeZeroScaleSwitchValue);
try
{
var parameter = new SqlParameter
{
DbType = DbType.DateTime2
};
if (setScale.HasValue)
{
parameter.Scale = (byte)setScale.Value;
}

var actualScale = (byte)typeof(SqlParameter).GetMethod("GetActualScale", BindingFlags.NonPublic | BindingFlags.Instance).Invoke(parameter, null);

Assert.Equal(outputScale, actualScale);
}

finally
{
SetLegacyVarTimeZeroScaleBehaviour(originalLegacyVarTimeZeroScaleSwitchValue);
}
}
}

[Fact]
public void SetLegacyVarTimeZeroScaleBehaviour_Defaults_to_True()
{
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)
{
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.
#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;
}
}
}
Loading