dotnet / SqlClient

Microsoft.Data.SqlClient provides database connectivity to SQL Server for .NET applications.
MIT License
842 stars 281 forks source link

Cannot write decimal.MaxValue if precision/scale are set on SqlParameter #1655

Open roji opened 2 years ago

roji commented 2 years ago

It seems like it's possible to write decimal.Max into a decimal(38, 4) column, as long as Precision and Scale aren't set on the parameter. The moment they are set, an exception is thrown. In EF Core, we always set the Precision/Scale because it's necessary for Always Encrypted to function correctly.

Exception:

Unhandled exception. System.OverflowException: Conversion overflows.
   at System.Data.SqlTypes.SqlDecimal.ToDecimal()
   at System.Data.SqlTypes.SqlDecimal.get_Value()
   at Microsoft.Data.SqlClient.TdsParser.AdjustDecimalScale(Decimal value, Int32 newScale)
   at Microsoft.Data.SqlClient.TdsParser.TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParameter param, MetaType mt, Byte options, SqlCommand command)
   at Microsoft.Data.SqlClient.TdsParser.TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, Int32 timeout, Boolean inSchema, SqlNotificationRequest notificationRequest, TdsParserStateObject stateObj, Boolean isCommandProc, Boolean sync, TaskCompletionSource`1 completion, Int32 startRpc, Int32 startParam)
   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.InternalExecuteNonQuery(TaskCompletionSource`1 completion, Boolean sendToPipe, Int32 timeout, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String methodName)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteNonQuery()
   at Program.<Main>$(String[] args) in /home/roji/projects/test/Program.cs:line 24
   at Program.<Main>$(String[] args) in /home/roji/projects/test/Program.cs:line 24
   at Program.<Main>$(String[] args) in /home/roji/projects/test/Program.cs:line 24
   at Program.<Main>(String[] args)

Full repro:

await using var conn = new SqlConnection("Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Encrypt=false");
await conn.OpenAsync();

await using var command = new SqlCommand(@"
DROP TABLE IF EXISTS data;
CREATE TABLE data (foo decimal(38,4))", conn);
await command.ExecuteNonQueryAsync();

// Works
using var cmd1 = new SqlCommand("INSERT INTO data (foo) VALUES (@p)", conn);
cmd1.Parameters.Add(new SqlParameter("p", decimal.MaxValue));
cmd1.ExecuteNonQuery();

// Throws
using var cmd2 = new SqlCommand("INSERT INTO data (foo) VALUES (@p)", conn);
cmd2.Parameters.Add(new SqlParameter("p", decimal.MaxValue) { Precision = 38, Scale = 4 });
cmd2.ExecuteNonQuery();

Originally reported in https://github.com/dotnet/efcore/issues/28240

DavoudEshtehari commented 2 years ago

Thanks @roji, We'll check and will get back to you.

k0c0w commented 1 year ago

It seems that some logic is incorrect in sqlbuffer or somewhere else. There were some issues years ago about precision and scale and some fixes have been made. Things that i have read about: decimal(38,20) to .net decimal , decimal(38,18) to net and one more wich i cant find now.

Seems some fixes helped, since i can save 2342342,35467895678945678956 now into (38,20), but i accidentally got new bug. i am using sql server and ef core 7.0.11. My database decimal type is decimal(38,20) and i am getting overflow exception when trying to save 9999999999999 (13 nines, notice that 12 nines is ok) for example.

i have quick look at sqlbuffer but could not find bug yet

DavoudEshtehari commented 1 year ago

For the record, when the scale is going to change SqlDecimal.AdjustScale is called. The return value is a SqlDecimal and an attempt to retrieve data from the Value property throws the overflow exception. This exception comes from System.Data.SqlTypes.SqlDecimal and I think the fix should be proposed from the related library.

IT-CASADO commented 7 months ago

I also run into this issue.

https://github.com/dotnet/efcore/issues/33109

Why is this only Low Priority. How can we workaround this?

A possible mentioned workaround here is not a viable solution. See the comment by @roji here.

IIRC it's important to set the precision/scale on the parameter, otherwise various issues can occur (e.g. with Always Encrypted); however, I agree it's odd that this causes the above failure.

shidcordero commented 1 month ago

It's been a while and haven't seen much update on this. We are also having issue storing decimal value with 28 precision on a decimal(38,10). Our code was heavy on EF and it will hard adding non-EF workaround. Any updates on this?

DavoudEshtehari commented 1 month ago

@shidcordero, @IT-CASADO this issue is related to underlying APIs and it would be better addressed in the related API, as I mentioned here.

@shidcordero If you have another issue, feel free to open a new issue and file your repro for further investigation.