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 Query: Cast from decimal to double not translated #10642

Closed
NArnott opened this issue Jan 4, 2018 · 11 comments
Closed

SQLite Query: Cast from decimal to double not translated #10642

NArnott opened this issue Jan 4, 2018 · 11 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-2.1 type-bug
Milestone

Comments

@NArnott
Copy link

NArnott commented Jan 4, 2018

When using Sqlite as a database for EFCore, the following expression doesn't work:

Where(x => x.Price > -1)

This expression will return items where Price is -5. One workaround is to use Decimal.Negate and reverse the expression, but that shouldn't be needed. Both methods work fine with using SQL as the database.

Steps to reproduce

Given Db Model Item:

class Item
{
    public long Id { get; set; }

    public string Name { get; set; }

    public decimal Price { get; set; }
}

and db context

class TestDbContext : DbContext
{
    public TestDbContext(DbContextOptions options) : base(options) { }

    public DbSet<Item> Items { get; set; }
}

and the following repo:

static async Task Main()
{
    var connection = new SqliteConnection("DataSource=:memory:");

    connection.Open();

    var options = new DbContextOptionsBuilder<TestDbContext>()
                        .UseSqlite(connection)
                        .Options;

    var context = new TestDbContext(options);

    context.Database.EnsureCreated();

    var items = new List<Item>
    {
        new Item{Id = 1, Name = "Item 1", Price = 100},
        new Item{Id = 2, Name = "Item 2", Price = 10},
        new Item{Id = 3, Name = "Item 3", Price = 0},
        new Item{Id = 4, Name = "Item 4", Price = -1},
        new Item{Id = 5, Name = "Item 5", Price = -5}
    };

    context.Items.AddRange(items);
    context.SaveChanges();

    var query1 = context.Items.Where(x => x.Price > 0).Select(x => x.Name);
    var results1 = await query1.ToArrayAsync();
    Console.WriteLine($"Items with price greater than 0 - {string.Join(',', results1)}");

    var query2 = context.Items.Where(x => x.Price > -1m).Select(x => x.Name);
    var results2 = await query2.ToArrayAsync();
    Console.WriteLine($"Items with price greater than -1 - {string.Join(',', results2)}");

    var query3 = context.Items.Where(x => decimal.Negate(x.Price) < 1).Select(x => x.Name);
    var results3 = await query3.ToArrayAsync();
    Console.WriteLine($"Items with price greater than -1 - {string.Join(',', results3)}");

    Console.ReadLine();
}

The result of query2 produces Item 1, 2, 3 and 5, where only Item 1, 2, 3 are expected.

Console.WriteLine("Hello World!");

Further technical details

EF Core version: 2.0.01
Database Provider: Microsoft.EntityFrameworkCore.Sqlite 2.0.1)
Operating system: Windows 10
IDE: Visual Studio 2017 15.5.2

@bricelam
Copy link
Contributor

bricelam commented Jan 8, 2018

Duplicate of #10265

You can mitigate it by casting to a double.

.Where(x => (double)x.Price > -1)

@bricelam
Copy link
Contributor

bricelam commented Jan 8, 2018

See the discussion in #10249 for more context.

@NArnott
Copy link
Author

NArnott commented Jan 10, 2018

@bricelam casting to double doesn't seem to work. In fact, it makes it worse. I get all 5 items returned instead of the expected 3 items (or the unexpected 4 items when I don't cast as double).

@bricelam
Copy link
Contributor

bricelam commented Jan 10, 2018

You're right. 🙁 The cast is getting lost. (Same for OrderBy().)

SELECT "x"."Name"
FROM "Items" AS "x"
WHERE "x"."Price" > -1

@bricelam bricelam reopened this Jan 10, 2018
@bricelam bricelam changed the title Where expression with negative Decimal values returns incorrect results SQLite Query: Cast from decimal to double not translated Jan 10, 2018
@bricelam
Copy link
Contributor

Workaroud

context.Items.FromSql("SELECT * FROM Items WHERE CAST(Price AS REAL) > -1").Select(x => x.Name);

@ajcvickers
Copy link
Member

Notes from triage: this is a case where the convert should not be removed since it contains semantic information needed for an appropriate translation. This is a specific case of the more general issue described in #10265 where, depending on how a type/value is mapped to the store, the translation may need to be changed or client eval may need to be triggered.

@smitpatel
Copy link
Contributor

#8861 added ability to introduce casting for projection but not for other places.

@divega
Copy link
Contributor

divega commented Nov 17, 2018

I discussed this issue with @smitpatel. It seems we have only thought about fixing for 3.0 by preserving the convert node if you have an explicit cast to double in .Where(x => (double)x.Price > -1) but without the cast this would still return incorrect results.

@smitpatel tells me that if there is no explicit cast, to avoid returning incorrect results (we can client-eval, throw or add the cast to REAL automatically), we would need additional information from the type mapping to know that decimal properties cannot be correctly compared on the server on SQLite, which is covered by #10249.

I have removed #10249 from the backlog to discuss it.

@bricelam
Copy link
Contributor

I think I already updated this to client-eval in 2.2 when there’s no cast.

@smitpatel
Copy link
Contributor

Generated SQL in new pipeline

SELECT "p"."ProductID", "p"."Discontinued", "p"."ProductName", "p"."SupplierID", "p"."UnitPrice", "p"."UnitsInStock"
FROM "Products" AS "p"
WHERE CAST("p"."UnitPrice" AS REAL) > -1.0

@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 5, 2019
@smitpatel smitpatel modified the milestones: 3.0.0, 3.0.0-preview6 Jun 5, 2019
smitpatel added a commit that referenced this issue Jun 5, 2019
smitpatel added a commit that referenced this issue Jun 6, 2019
@bricelam
Copy link
Contributor

bricelam commented Jun 6, 2019

My hero! lol, I'm so happy with all the improvements around types in 3.0

@ajcvickers ajcvickers modified the milestones: 3.0.0-preview6, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-2.1 type-bug
Projects
None yet
Development

No branches or pull requests

6 participants