fsprojects / SQLProvider

A general F# SQL database erasing type provider, supporting LINQ queries, schema exploration, individuals, CRUD operations and much more besides.
https://fsprojects.github.io/SQLProvider
Other
580 stars 146 forks source link

Comparing on string results System.InvalidOperationException #328

Closed s952163 closed 8 years ago

s952163 commented 8 years ago

Description

When doing comparison on date strings in SQLite, greater than or smaller than results in:

System.InvalidOperationException: The binary operator GreaterThan is not defined for the types 'System.String' and 'System.String.

Repro steps

#if INTERACTIVE
#I @"..\packages\SQLProvider.1.0.31\lib"
#r "FSharp.Data.SqlProvider.dll" 
#I @"..\packages\System.Data.SQLite.Core.1.0.102.0\lib\net46"
#r "System.Data.SQLite.dll"
#I @"..\packages\System.Data.SQLite.Linq.1.0.102.0\lib\net46"
#r "System.Data.SQLite.Linq.dll"
#r "System.Transactions.dll"
#endif

open System
open FSharp.Data.Sql
open System.Data.SQLite
open System.Data.SQLite.Linq

[<Literal>]
let connectionString = "Data Source="+ @"C:\tmp\databaseFile.db3"
[<Literal>]
let resolutionPath = __SOURCE_DIRECTORY__ + @"..\..\packages\System.Data.SQLite.Core.1.0.102.0\lib\net46"

type sql = SqlDataProvider<
                Common.DatabaseProviderTypes.SQLITE, 
                connectionString, 
                ResolutionPath = resolutionPath, 
                CaseSensitivityChange = Common.CaseSensitivityChange.ORIGINAL>

//Create test table
let mkTable() =
    let conn =  new SQLiteConnection(@"Data Source=C:\tmp\databaseFile.db3")
    conn.Open()
    let sql = @"CREATE TABLE IF NOT EXISTS [Table6] (
                          [ID] INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
                          [Date1] TEXT NOT NULL)"
    let cmd = new SQLiteCommand(conn)
    cmd.CommandText <-sql    // Set CommandText to our query that will create the table
    cmd.ExecuteNonQuery()  |> ignore
    conn.Close()

mkTable()

//Create test data:
let mkDates x =
    let today = System.DateTime(2016,12,3,0,0,0) 
    [for i in 1..x -> (today.AddDays(float i))]

let dts = mkDates 10

//Connect:
let ctx = sql.GetDataContext() 
let table6 = ctx.Main.Table6

//Insert data:
dts  |> List.iter (fun x -> 
                            let row = table6.Create()
                            row.Date1 <- x.ToString("yyyy-MM-dd hh:mm:ss"))
ctx.SubmitUpdates()

//Insert data:
dts  |> List.iter (fun x -> 
                            let row = table6.Create()
                            row.Date1 <- x.ToString("yyyy-MM-dd hh:mm:ss"))
ctx.SubmitUpdates()

//Query that works:
query {                                         
        for r in table6 do
            where (r.Date1 = "2016-12-08 12:00:00")
            select (r.Date1)                                
            } |> Seq.toList
//val it : string list = ["2016-12-08 12:00:00"]

//Query that doesn't work:
query {                                         
        for r in table6 do
            where (r.Date1 > "2016-12-08 12:00:00")
            select (r.Date1)                                
            } |> Seq.toList
//System.InvalidOperationException: The binary operator GreaterThan is not defined for the types 'System.String' and 'System.String'.

Expected behavior

Sort based on string comparison.

Actual behavior

System.InvalidOperationException: The binary operator GreaterThan is not defined for the types 'System.String' and 'System.String'.
>    at System.Linq.Expressions.Expression.GetUserDefinedBinaryOperatorOrThrow(ExpressionType binaryType, String name, Expression left, Expression right, Boolean liftToNull)
   at System.Linq.Expressions.Expression.GreaterThan(Expression left, Expression right, Boolean liftToNull, MethodInfo method)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext@529-4.Invoke(Tuple`2 tupledArg)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp)
   at Microsoft.FSharp.Primitives.Basics.List.mapToFreshConsTail[a,b](FSharpList`1 cons, FSharpFunc`2 f, FSharpList`1 x)
   at Microsoft.FSharp.Primitives.Basics.List.map[T,TResult](FSharpFunc`2 mapping, FSharpList`1 x)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprsToLinq(ConvEnv env, FSharpList`1 es)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp)
   at Microsoft.FSharp.Primitives.Basics.List.map[T,TResult](FSharpFunc`2 mapping, FSharpList`1 x)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprsToLinq(ConvEnv env, FSharpList`1 es)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.ConvExprToLinqInContext(ConvEnv env, FSharpExpr inp)
   at Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.EvaluateQuotation(FSharpExpr e)
   at Microsoft.FSharp.Linq.QueryModule.EvalNonNestedInner(CanEliminate canElim, FSharpExpr queryProducingSequence)
   at Microsoft.FSharp.Linq.QueryModule.clo@1735-1.Microsoft-FSharp-Linq-ForwardDeclarations-IQueryMethods-Execute[a,b](FSharpExpr`1 )
   at <StartupCode$FSI_0010>.$FSI_0010.main@() in C:\Users\s9951_000\Documents\Visual Studio 2015\Projects\SqLite-v2\SqLite-v2\Script1.fsx:line 86
Stopped due to error

Related information

.NET 4.6

Thorium commented 8 years ago

Thanks. This is actually very easy to reproduce with our current SQLite unit tests. And there are fixes... But the stacktrace is not ours, so needs a bit more exploration how we could get workaround.

Hmm... If the standard SQLite doesn't have datetime... One hack in the meanwhile would be saving datetime values also to int column and compare them.

Thorium commented 8 years ago

With SQLite (at least version 1.0.102) you should have an option to create DateTime data-type. Would this be enough?

s952163 commented 8 years ago

@Thorium Again, thanks for looking into this and the prompt reply. Indeed, you have referenced my own question/answer on SO. It is generally preferable to use DateTime, while SQLite doesn't have a concept of DateTime, it's very flexible in allowing basically any type as a column. So as long as Linq or SQLProvider can correctly translate back to .NET DateTime, it will work. However, using DateTime can be also tricky: see this [ Question (keep quiting myself ;-). Because SQLite tends to deal with UTC and although it will happily store anything, it will give back UTC and that can be confusing, when doing comparison. So the idea of storing DateTime as string (int can have its own issues) is a quite asafe and portable way to deal with Dates. I also noticed that the stack trace goes to IQueryable and FSharp.Linq. If the issue is actually even deeper than a fix is even more preferable.

Thorium commented 8 years ago

What would you think of this fix candidate:

query {                                         
   for r in table6 do
   where (DateTime.Parse(r.Date1) > DateTime.Parse("2016-12-08 12:00:00"))
   select (r.Date1)                                
   } |> Seq.toList

...and then we expect that the database is actually capable of handling implicit conversion? So the SQL will be just as-is, not any casting, as casting may lead to not hitting indexes and other kind of complex problems that you can see in EntityFramework.

s952163 commented 8 years ago

Nice! This is not valid Linq right now, correct? I think this should work for the most common case of storing dates as text so I'm all for it. The issue of other types of string comparisons will remain I suspect but as I cannot offer any deep insight on what was going wrong in the first place this looks to me a smart workaround.

Thorium commented 8 years ago

Nuget package 1.0.32 has this feature.

s952163 commented 8 years ago

:+1: Will test. thx. Works Nice! :+1: