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

5.2.0 broke DATETIMEOFFSET(1..4) in TVPs #2423

Closed
cmeeren opened this issue Mar 21, 2024 · 7 comments · Fixed by #2453
Closed

5.2.0 broke DATETIMEOFFSET(1..4) in TVPs #2423

cmeeren opened this issue Mar 21, 2024 · 7 comments · Fixed by #2453
Projects
SqlClient Triage Board SqlClient v5.2.1 hotfix

Comments

@cmeeren
Copy link

cmeeren commented Mar 21, 2024

From 5.2.0, an exception is thrown when using DATETIMEOFFSET(n) in a TVP if n is 1, 2, 3, or 4.

Repro code:

open System
open System.Data
open Microsoft.Data.SqlClient
open Microsoft.Data.SqlClient.Server

type DateTimeOffsetList(datetimeoffset: DateTimeOffset) as this =
    inherit SqlDataRecord([| SqlMetaData("datetimeoffset", SqlDbType.DateTimeOffset, 0uy, 1uy) |])
    do this.SetValues(datetimeoffset) |> ignore

let param =
    SqlParameter(
        "@params",
        SqlDbType.Structured,
        TypeName = "dbo.DateTimeOffsetList",
        Value = [ DateTimeOffsetList(datetimeoffset = DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero)) ]
    )

let conn = new SqlConnection("Data Source=.;Initial Catalog=TvpRepro;Integrated Security=True;Encrypt=False")
conn.Open()
let cmd = conn.CreateCommand()
cmd.CommandText <- "SELECT * FROM @params"
cmd.Parameters.Add(param) |> ignore
cmd.ExecuteReader() |> ignore

Running this as-is requires a database TvpRepro with the following TVP:

CREATE TYPE dbo.DateTimeOffsetList AS TABLE
(
  [Value] DATETIMEOFFSET(1) NOT NULL
)

When run, it produces this exception:

Unhandled exception. Microsoft.Data.SqlClient.SqlException (0x80131904): The incoming tabular data stream (TDS) remote procedure call (RPC) protocol stream is incorrect. Table-valued parameter 0 (""), row 1, column 1: The supplied value is not a valid instance of data type datetimeoffset. Check the source data for invalid values. An example of an invalid value is data of numeric type with scale greater than precision.
The statement has been terminated.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, SqlCommand command, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
   at Microsoft.Data.SqlClient.SqlDataReader.get_MetaData()
   at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boo
   at Microsoft.Data.SqlClient.SqlDataReader.get_MetaData()
   at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteReader()
   at <StartupCode$TvpRepro>.$Program.main@() in C:\Users\cmeer\Source\Repos\TvpRepro\TvpRepro\Program.fs:line 23
ClientConnectionId:fa0131e2-6148-4429-b44e-8d90ecdeba22
Error Number:8043,State:1,Class:16

Strictly speaking, this only happens if the scale set in SqlMetaData is 1, 2, 3, or 4. It seems to be unrelated to the number used in the SQL type definition (though I assume they should always match).

This error does not occur before 5.2.0.

Further technical details

Microsoft.Data.SqlClient 5.2.0
.NET 8
SQL Server Developer 15.0.2104.1
Windows 11
cmeeren pushed a commit to cmeeren/Facil that referenced this issue Mar 21, 2024
Revert "Update Microsoft.Data.SqlClient from 5.1.4 to 5.2.0"
14f9ea5 
This reverts commit 9e38107.

Avoids upstream bug: dotnet/SqlClient#2423
@JRahnama JRahnama added this to Needs triage in SqlClient Triage Board via automation Mar 21, 2024
@DavoudEshtehari DavoudEshtehari moved this from Needs triage to Needs Investigation in SqlClient Triage Board Mar 26, 2024
@DavoudEshtehari DavoudEshtehari added 🐛 Bug! Something isn't right ! and removed untriaged 🐛 Bug! Something isn't right ! labels Mar 26, 2024
@DavoudEshtehari
Copy link
Member

Thank you, we'll check it out.

@arellegue
Copy link
Contributor

@cmeeren. Just needed to clarify the repro. You mentioned in the repro that "Running this as-is requires a database TvpRepro with the following TVP:"

CREATE TYPE dbo.DateTimeOffsetList AS TABLE
(
[Value] DATETIMEOFFSET(1) NOT NULL
)

However, in the command text "SELECT * FROM @params " it seems it is querying a user defined table type and not an actual instance of a table. Do you actually intend to query an actual instance of a table using dbo.DateTimeOffsetList type?

@cmeeren
Copy link
Author

cmeeren commented Apr 4, 2024 •
edited

However, in the command text "SELECT * FROM @params " it seems it is querying a user defined table type and not an actual instance of a table.

Not sure what you mean by "instance of a table". I don't mention a table anywhere in my post. If you are referring to the name TvpRepro, note that I mention this is the name of the database, not a table. (It's used in the connection string in the repro code, and it is of course trivial to replace.)

Was that clarifying?

@arellegue
Copy link
Contributor

arellegue commented Apr 4, 2024 •
edited

@cmeeren. Using version 5.1 displayed the parameter value without issues.

let conn = new SqlConnection("Data Source=.;Initial Catalog=TvpRepro;Integrated Security=True;Encrypt=False")
conn.Open()
let cmd = conn.CreateCommand()
cmd.CommandText <- "SELECT * FROM @params"
cmd.Parameters.Add(param) |> ignore
let result = cmd.ExecuteScalar() 
printfn "%A" result   

1/1/2000 12:00:00 AM +00:00

The root cause of regression is being investigated.

@arellegue
Copy link
Contributor

The issue was introduced in enhancement #2170.

@saitama951, do you have a unit test for the above enhancement you can share? If not, would you mind testing locally when the fix on this issue is done to ensure the enhancement is not affected by the change?

@arellegue arellegue mentioned this issue Apr 8, 2024
Merged
@saitama951
Copy link
Contributor

saitama951 commented Apr 9, 2024 •
edited

@arellegue I didn't have a specific unit test for this enhancement. apparently, I tested this against the manual and functional test cases present in SqlClient (to see nothing breaks with x86 and s390x). Sure I will test it out.

@saitama951
Copy link
Contributor

saitama951 commented Apr 9, 2024 •
edited

I think there are different lengths for SqlDbType and probably the extra bytes written to the stream might be the cause ( in some cases)?
I would suggest we use the old way.

--- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsValueSetter.cs
+++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsValueSetter.cs
@@ -697,15 +697,17 @@ namespace Microsoft.Data.SqlClient
             short offset = (short)value.Offset.TotalMinutes;
 
 #if NETCOREAPP
-            Span<byte> result = stackalloc byte[9];
+            Span<byte> result = stackalloc byte[5];
             BinaryPrimitives.WriteInt64LittleEndian(result, time);
-            BinaryPrimitives.WriteInt32LittleEndian(result.Slice(5), days);
-            _stateObj.WriteByteSpan(result.Slice(0, 8));
+            _stateObj.WriteByteSpan(result.Slice(0, length-5));
+            BinaryPrimitives.WriteInt32LittleEndian(result, days);
+            _stateObj.WriteByteSpan(result.Slice(0, 3));
 #else
             _stateObj.WriteByteArray(BitConverter.GetBytes(time), length - 5, 0); // time
             _stateObj.WriteByteArray(BitConverter.GetBytes(days), 3, 0); // date
 #endif

@DavoudEshtehari DavoudEshtehari linked a pull request Apr 12, 2024 that will close this issue
Fix | Fix DateTimeOffset size in TdsValueSetter.cs class file. #2453
Merged
SqlClient Triage Board automation moved this from Needs Investigation to Closed May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
SqlClient Triage Board
  
Closed
SqlClient v5.2.1 hotfix
To do
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix | Fix DateTimeOffset size in TdsValueSetter.cs class file. arellegue/SqlClient
5 participants
@cmeeren @JRahnama @saitama951 @DavoudEshtehari @arellegue