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

[EamonHetherton]

The pull requests addresses #1380 by introducing and opt-in context switch "Switch.Microsoft.Data.SqlClient.LegacyVarTimeZeroScaleBehaviour" to change the existing behaviour and actually emit a zero scale parameter when the users has set the scale-0

[Wraith2]

It wasn't stuck in limbo it was decided against, see #1380 (comment)

The only way you're going to get the change implemented is with an appcontext opt-in, if you include that then it might be includable. You'll need to add tests. You'll need to change the documentation.

[EamonHetherton]

It wasn't stuck in limbo it was decided against, see #1380 (comment)

The only way you're going to get the change implemented is with an appcontext opt-in, if you include that then it might be includable. You'll need to add tests. You'll need to change the documentation.

Apologies, I'd assumed that this was blocked based on this #1381 (comment)

I'm really out of options for how to mitigate the performance impact of this short of moving off MSSQL or forking this library. I really don't want to do either :(

I'll chip away and attempt to make this change based on an app context switch, try to get some testing around it and update the docs and then and see if I can get some traction.

Also, it looks like the failed checks are unrelated to this pull request as best that I can tell, I'm really not sure how to get the checks to pass.

[EamonHetherton]

It looks like the explicit calling out of this behaviour in the docs for the previous library System.Data.SqlClient as referred to here: #1380 (comment) are no longer in the docs for Microsoft.Data.SqlClient. Therefore I don't think there are any doc changes to make as part of this pull request. Still working on tests :)

[EamonHetherton]

@Wraith2 pull request updated with context switch and tests. I don't believe there are documentation updates required for the Scale parameter itself as noted above, but the existing context switches do appear in documentation here: https://learn.microsoft.com/en-us/sql/connect/ado-net/appcontext-switches?view=sql-server-ver16 but I'm not sure where that lives. Any guidance would be appreciated.

[EamonHetherton]

@Wraith2 Incidentally I noticed that the UseMinimumLoginTimeout context switch changed default behaviour in the last refactor of LocalAppContextSwitches in November 2023 and now returns a default value of "false" though the triple slash summary still says it should have a default of "true". The documentation @ https://learn.microsoft.com/en-us/sql/connect/ado-net/appcontext-switches?view=sql-server-ver16 does not give any guidance on default value but it looks to me like this default value change was unintended, but I think you would be better placed to determine that.

[Wraith2]

Can you open a new issue with the details on that default so it can be reviewed please.

[EamonHetherton]

Can you open a new issue with the details on that default so it can be reviewed please.

issue #2415 with fix in #2416

[EamonHetherton]

Is there anything more I can do to move this along? failing that is there anywhere for me to get access to the pull request nuget package that was built "Microsoft.Data.SqlClient.6.0.0-pull.107808"?

[Wraith2]

No. You just wait for the team to get to it.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.70%. Comparing base (c33bde2) to head (ab11ffc).

❗ Current head ab11ffc differs from pull request most recent head 546c10a. Consider uploading reports for the commit 546c10a to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2411      +/-   ##
==========================================
+ Coverage   72.66%   72.70%   +0.04%     
==========================================
  Files         313      313              
  Lines       61737    61747      +10     
==========================================
+ Hits        44861    44895      +34     
+ Misses      16876    16852      -24     

Flag	Coverage Δ
addons	92.88% <ø> (ø)
netcore	76.99% <100.00%> (+<0.01%)	⬆️
netfx	70.42% <100.00%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[EamonHetherton]

6 months later this is still waiting to be reviewed/approved....

[cheenamalhotra]

Please update PR description to include the behavior change and introduction of new App Context switch for documentation purposes.

[EamonHetherton]

suggested documentation for new AppContext switch to update this

Allow explicit zero scale for VarTime parameter types

[!INCLUDE appliesto-netfx-netcore-netst-md]

(Available starting with version 6.0.0)

When using parameters with VarTime datatypes (DateTime2, DateTimeOffset, Time), the library will only emit the scale if it is non-zero regardless of the user explicitly setting the Scale=0. See https://learn.microsoft.com/en-us/dotnet/api/system.data.sqlclient.sqlparameter.scale?view=netframework-4.8.1#remarks

In order to emit a zero scale, you can now set the following AppContext switch to false:

AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.LegacyVarTimeZeroScaleBehaviour", false);

and then explicitly set the parameter scale to zero

var parameter = new SqlParameter
{
    ParameterName = "@p1",
    DbType = DbType.DateTime2,
    Scale = 0
};

which will emit the equivalent SQL parameter as

DECLARE @p1 datetime2(0)