11

Closed

generated SQL truncates (not rounds) decimal values based on target column scale in EDM metadata

description

This manifested as well in a different issue I just filed (http://entityframework.codeplex.com/workitem/734)

Created this test table:

CREATE TABLE [dbo].[DecimalTestingTable](
[Dec1] [decimal](6, 1) NULL,
[Dec2] [decimal](6, 2) NULL,
[Dec3] [decimal](6, 3) NULL,
[Dec4] [decimal](6, 4) NULL,
[Dec5] [decimal](6, 5) NULL,
[NN1] [decimal](6, 1) NOT NULL,
[NN2] [decimal](6, 2) NOT NULL,
[NN3] [decimal](6, 3) NOT NULL,
[NN4] [decimal](6, 4) NOT NULL,
[NN5] [decimal](6, 5) NOT NULL
)

Inserting these values via manual SQL:

insert into DecimalTestingTable values
(
0.88888888,
0.88888888,
0.88888888,
0.88888888,
0.88888888,

0.88888888,
0.88888888,
0.88888888,
0.88888888,
0.88888888
)

This results in SQL rounding when it writes the column values, such that selecting back out of the table gives these values:

Dec1 Dec2 Dec3 Dec4 Dec5 NN1 NN2 NN3 NN4 NN5
0.9 0.89 0.889 0.8889 0.88889 0.9 0.89 0.889 0.8889 0.88889

However, if I have a model (code first in this case, although I would guess it doesn't matter) that includes the right precision/scale info for the columns, the values are modified on the SQL client side instead of being sent 'whole'/unmodified to SQL server.
var context = new DecimalTestingContext();
context.DecimalTestingTables.Add(new DecimalTestingTable()
{
    Dec1 = 0.88888888M,
    Dec2 = 0.88888888M,
    Dec3 = 0.88888888M,
    Dec4 = 0.88888888M,
    Dec5 = 0.88888888M,
    NN1 = 0.88888888M,
    NN2 = 0.88888888M,
    NN3 = 0.88888888M,
    NN4 = 0.88888888M,
    NN5 = 0.88888888M,
});
context.SaveChanges();
If you don't modify the reverse-engineered model, then you'll get this generated SQL (this is what 734 is about, just including here in case others try to repro this and see this SQL)

exec sp_executesql N'insert [dbo].[DecimalTestingTable]([NN1], [NN2], [NN3], [NN4], [NN5], [Dec1], [Dec2], [Dec3], [Dec4], [Dec5])
values (@0, @1, @2, @3, @4, @5, @6, @7, @8, @9)
',N'@0 decimal(18,2),@1 decimal(18,2),@2 decimal(18,2),@3 decimal(18,2),@4 decimal(18,2),@5 decimal(18,2),@6 decimal(18,2),@7 decimal(18,2),@8 decimal(18,2),@9 decimal(18,2)',@0=0.88,@1=0.88,@2=0.88,@3=0.88,@4=0.88,@5=0.88,@6=0.88,@7=0.88,@8=0.88,@9=0.88 If the model includes the correct precision/scale information like this:
this.Property(t => t.Dec1).HasColumnName("Dec1").HasPrecision(6, 1);
this.Property(t => t.Dec2).HasColumnName("Dec2").HasPrecision(6, 2);
this.Property(t => t.Dec3).HasColumnName("Dec3").HasPrecision(6, 3);
this.Property(t => t.Dec4).HasColumnName("Dec4").HasPrecision(6, 4);
this.Property(t => t.Dec5).HasColumnName("Dec5").HasPrecision(6, 5);
this.Property(t => t.NN1).HasColumnName("NN1").HasPrecision(6, 1);
this.Property(t => t.NN2).HasColumnName("NN2").HasPrecision(6, 2);
this.Property(t => t.NN3).HasColumnName("NN3").HasPrecision(6, 3);
this.Property(t => t.NN4).HasColumnName("NN4").HasPrecision(6, 4);
this.Property(t => t.NN5).HasColumnName("NN5").HasPrecision(6, 5);

then we still end up with SQL-client-side truncation of the value instead of either 1) rounding like SQL Server will do (which might need to check what 'mode' SQL is in, assuming this behavior is settable) or 2) (IMHO, preferred) just sending the unmodified value through to the server so we get whatever the server-defined behavior is (by default, appears to be rounding).

I'd imagine modifying the value on the client side before sending it to the server is done with a goal of keeping the client and server in sync with what value is stored without re-querying the just-inserted-or-updated data, so keeping that behavior might be necessary, but if so, it seems like worst-case the model should support including the truncation-versus-rounding behavior so I could either use a new overload of HasPrecision that accepted a 'bool shouldRoundInsteadOfTruncate' or a new model configuration .ShouldRoundToScale() (or whatever it would be called) I could add after the HasPrecision.

My apologies if the 'round instead of truncate when modifying the value before sending to the server' behavior is already configurable and I just missed it. :)

Thanks!
Closed May 1, 2013 at 9:19 PM by maumar
verified, closing

comments

ajcvickers wrote Dec 17, 2012 at 10:09 PM

Thanks for reporting this issue. Unfortunately I don't think we can change the default behavior since it would break apps that have been written to rely on the existing behavior.

That being said, we would like to make this something that can be configured either as a global option or as configuration on the individual column mappings.

We haven't discussed in detail which of these options are best or what the API surface would look like. We plan to do this when we start working on the bug, which may or may not end up being done as part of EF6. However, if anybody is interested in contributing code for this then we would certainly welcome that and would be very happy to discuss design with that person. Just let us know, either here, or as part of a discussion thread.

Thanks,
Arthur

ajcvickers wrote Dec 18, 2012 at 7:33 PM

Hi James,

I've discussed this briefly with the team and the feeling is that a global option accessed from DbContext.Configuration should be fine. Something like "RoundDecimalsToScale". Switching this on would cause all decimals to be rounded during updates instead of being truncated.

It doesn't seem very likely that an application will need to round some decimals and truncate others, and if an application does want to do this then it is probably better done in business logic rather than in the data layer. Therefore we don't think the value of allowing individual columns to be configured outweighs the extra complexity (API, annotations, in the model, etc.) involved. Also, allowing individual columns to be configured would require changes to the designer when using one of the EDMX workflows.

If you choose to contribute this, then please follow the instructions here: http://entityframework.codeplex.com/wikipage?title=Contributing In particular, don't forget to submit a Contributor License Agreement and also please make sure to add tests.

Thanks,
Arthur

jmanning wrote Dec 18, 2012 at 9:05 PM

Just in case someone else runs across this work item, one possible workaround that seems to work ok for me is to just 'lie' in the HasPrecision call (unfortunately, yes, you'd have to do this for every decimal column you care about this behavior in) to have a scale 1 higher than what it actually is - that way an additional digit it sent to SQL server, where it will be rounded instead of truncated (at least that's the behavior I see on my SQL Server 2012 instance, I'm assuming it's a configurable setting somewhere but I haven't looked).

Thanks for the info, Arthur! Hopefully I'll be able to give the global config change a shot soon. :)

erikj999 wrote Jan 14, 2013 at 11:44 PM

But the default behavior today is plainly a bug. The right thing to do is to pass the decimal value straight through to the database and let it handle the rounding according to the precision rules of the underlying type. Otherwise EF is simply applying undocumented and unjustified side-effects to data.

ajcvickers wrote Apr 29, 2013 at 7:35 PM

Fixed in 0f1b6c7c0757

ThwartingSupermanIII (Add flag to change truncation behavior for decimal parameters)

This turned out to be an issue with the SQL provider, which meant that the idea of having a flag on the context was not really appropriate. The flag has instead been added as a static on SqlProviderServices, with the idea being that an application that depends on this can set the flag at startup and then forget about it, but we can discuss if there is a better place for it.

The underlying issue is in SQL Client where if Scale is set on a SqlParameter object then it will cause the value to be truncated. The SqlParameter documentation says that the truncation behavior should not be relied upon, but it seems very unlikely that it will change. The recommended approach is to not set Scale at all for input parameters, which is what EF now does when the flag is set.

We may want to discuss again the value of changing the default to not truncate. This is a breaking change for anyone relying on the old behavior that would be hard to detect. On the other hand, not changing the default will likely result in hard to find bugs in new applications and may also fix some existing applications that are not aware of and have not tested for the truncation behavior.

The SQL Compact provider does not appear to have this issue. However, it does always truncate "money" columns, but this appears to be below the level of the provider and there doesn't seem to be a way to influence it from EF.

These comments were discussed and the conclusion was that we should not change the default based on:
  • There is no strong contract anywhere that says that storing a decimal value on a decimal column with less scale should round the value rather than truncating it, and therefore there are possibly customers/applications out there that expect truncation to be the behavior and those aren’t complaining today but they could start complaining if we broke them
  • A flag on SqlProviderServices is an adequate way to present a configuration option that is scoped to a specific provider

ajcvickers wrote May 1, 2013 at 5:14 PM

Additional fix in: LexLutherStrikesBack (Fix additional bug around decimal scale/precision)

In the previous fix the Scale was not being set put Precision was still being set. It turns out that in this combination if the total number of digits in the decimal is greater than the set precision, then SQL Client throws. The fix is to set neither, in which case the decimal value is passed through unchanged.

michaelaird wrote Jan 20 at 9:46 PM

I can't seem to find where to set this option. Can you provide a code sample?

ajcvickers wrote Jan 23 at 5:30 PM

@michaelaird: The flag is "TruncateDecimalsToScale" on "System.Data.Entity.SqlServer.SqlProviderServices" which is in the EntityFramework.SqlServer assembly.