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

SQLite: decimal literals formatted incorrectly #8205

Closed
Mats391 opened this issue Apr 18, 2017 · 9 comments
Closed

SQLite: decimal literals formatted incorrectly #8205

Mats391 opened this issue Apr 18, 2017 · 9 comments

Comments

@Mats391
Copy link

Mats391 commented Apr 18, 2017

When I insert a decimal value into a Sqlite database using EntityFramework Core, it will save it in a Text column. This is fine and prevents loss of data, but sometimes makes you unable to find a value you added before

Steps to reproduce

  • Create Sqlite DbContext
  • Insert Entity with a decimal value without decimal places
  • SaveChanges
  • Try to find by inserted value
using(var db = new MyDbContext()) {
    db.Database.EnsureCreated();
    db.Add( new MyEntity() { Value = 3m } );
    db.SaveChanges();

    // Creates error
    db.Entities.First( x => x.Value == 3m );
}

It adds the value as "3" into the database, but uses "3.0" in the query. It seems to not use the same ToString()

Further technical details

EF Core version: 1.1.1
Database Provider: Microsoft.EntityFrameworkCore.Sqlite
Operating system: Win7 x86
IDE: Visual Studio 2017

Edit:
This works for some reason:

using(var db = new MyDbContext()) {
    db.Database.EnsureCreated();
    decimal value = 3m;
    db.Add( new MyEntity() { Value = value } );
    db.SaveChanges();

    // Works as expected
    var y = db.Entities.First( x => x.Value == value );
}
@bricelam
Copy link
Contributor

bricelam commented Apr 18, 2017

Looks like we need to add the following.

partial class SqliteSqlGenerationHelper
{
    protected overrride string DecimalFormat => "G";
}

BUT this is only the tip of the iceberg. Any arithmetic operation will coerce the values into doubles before performing the operation. This could yield lossy results. In general, using decimal values (and any non-SQLite-primitive type like DateTime, DateTimeOffset, and TimeSpan) in an expression that gets evaluated on the server should be carefully reviewed for potential errors due to coercion.

@bricelam
Copy link
Contributor

Oh wait, the above code probably isn't sufficient. They need to be TEXT literals.

@bricelam
Copy link
Contributor

We should review all literals too--we had anther issue with Guid in 1.0.0.

@ajcvickers ajcvickers modified the milestones: 2.0.0, 2.0.0-preview1 Apr 19, 2017
@bricelam
Copy link
Contributor

bricelam commented Apr 21, 2017

It looks like double and float literals are also wrong for certain values: (#8270)

SQLite Error 1: 'unrecognized token: "1.79769313486232E+308E0"'.

@bricelam bricelam changed the title [Sqlite] Decimal values SQLite: decimal literals formatted incorrectly May 2, 2017
@bricelam bricelam assigned smitpatel and unassigned bricelam Jun 23, 2017
@bricelam
Copy link
Contributor

@smitpatel the logic used by Microsoft.Data.Sqlite is value.ToString(CultureInfo.InvariantCulture) quoted as a string literal.

@smitpatel smitpatel added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels Jun 29, 2017
@smitpatel
Copy link
Contributor

This should be fixed in SQLite to allow lossless arithmetic operations.

@bricelam
Copy link
Contributor

lossless less lossy

@bricelam
Copy link
Contributor

Fixed by aspnet/Microsoft.Data.Sqlite#381

@bricelam bricelam assigned bricelam and unassigned smitpatel Jun 30, 2017
@bricelam bricelam added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. breaking-change labels Jun 30, 2017
@bricelam
Copy link
Contributor

This issue was moved to aspnet/Microsoft.Data.Sqlite#382

@bricelam bricelam removed this from the 2.0.0 milestone Jun 30, 2017
@bricelam bricelam removed their assignment Jun 30, 2017
@bricelam bricelam removed breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug labels Jun 30, 2017
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants