mfenniak / rethinkdb-net

A C# / .NET client driver for RethinkDB.
Other
247 stars 37 forks source link

Support changing F# record fields in Update functions #195

Open spiffytech opened 9 years ago

spiffytech commented 9 years ago

I'm trying to write an Update function that alters the field in an F# record. Like this:

conn.Run(Query.Db("sleep_log").Table<Sleep>("sleeps").Update(fun sleep -> {sleep with timeToBed = dt}))

This produces the following exception:

System.AggregateException: One or more errors occurred ---> System.InvalidOperationException:
Failed to perform client-side evaluation of expression tree node; often this is caused by
refering to a server-side variable in a node that is only supported w/ client-side evaluation --->
System.InvalidOperationException: variable 'sleep' of type 'sleep_log.Migrations+Sleep'
referenced from scope '', but it is not defined

If I change my query to do something besides update the field, it works. Unfortunately, this is the way to update records in F#, so this bug puts a damper on my ability to use RethinkDB in my app.

mfenniak commented 9 years ago

Hm, I'm not exactly up-to-speed on F# these days.

It looks like the F# code here compiles into a LINQ expression tree that calls the constructor of the record type (eg. Sleep). For example, in this F# code:

// Learn more about F# at http://fsharp.net
// See the 'F# Tutorial' project for more help.
open System.Runtime.Serialization

[<DataContract>]
type Result = {
    [<field: DataMember(Name="code") >]
    Code:string
    [<field: DataMember(Name="message") >]
    Message:string
}

let blah t =
    printfn "%A" t.Code

[<EntryPoint>]
let main argv = 
    let p = { Result.Code = "Code"; Message = "Message" }
    blah p
    let q = { p with Code = "Code #2" }
    blah q
    0

The "main" method gets translated into IL that is basically the same as this C# code:

[EntryPoint]
public static int main (string[] argv)
{
    Program.Result t = new Program.Result ("Code", "Message");
    Program.blah (t);
    Program.Result t2 = new Program.Result ("Code #2", "Message");
    Program.blah (t2);
    return 0;
}

That fits with F#'s general principal of using immutable objects, which makes sense... but that's tricky to support. What constructors would we convert into RethinkDB objects, and what field names would they have? We could probably make it so that you could "register" the record types you're using, and create the appropriate expression converters to convert a constructor call into making those objects server-side. But it wouldn't be a small amount of work.

A possible workaround here would be to use F# records that are marked as CLIMutable. I'm not sure what that does exactly, but, reading about it suggests that it creates mutable record types instead of immutable ones, so it might create code that's compatible with our existing expression converters.

mfenniak commented 9 years ago

Looks like we could reflect on a given type that the user requests to be registered, and find all the properties that have Microsoft.FSharp.Core.CompilationMappingAttribute. The property name, plus the SequenceNumber from the attribute, could be used to know how to convert the constructor call on that type into a RethinkDB object. Then that information would need to be registered on an instance of DefaultExpressionConverterFactory with RegisterNewExpressionMapping, which would then convert the LINQ expression into a RethinkDb.Term object.

Since we'd have to refer to to Microsoft.FSharp.Core, we'd probably want to do this in an add-on assembly (eg. rethink-db-net-fsharp) so that the F# lib isn't a dependency that's forced upon all users.

I guess... long story short... it definitely seems possible to support this. If I had time right now, I'd be all over implementing this, because it's interesting. :-) If someone else wants to follow my thoughts and see if they go somewhere productive, I'd be happy to help and code review.

Thanks for the bug report, @spiffytech; definitely an issue, unfortunately not a simple one to fix.

spiffytech commented 9 years ago

OK, thanks.

I tried adding the CLIMutable decorator to my F# record type and still got the original exception.

karlgrz commented 9 years ago

I should be able to take a look at this tonight. This is -very- interesting to me, as well ;-)

Hopefully I can make some progress.

karlgrz commented 9 years ago

Hey @spiffytech, I started looking into this in the morning. I'm by no means a F# expert, so I think it would be helpful to know how you are using the library in your code when it works for reads.

I ran this against a local RethinkDB instance and I was able to connect and retrieve the correct number of objects, but nothing is getting serialized, even for read only.

// Learn more about F# at http://fsharp.net
// See the 'F# Tutorial' project for more help.
open Microsoft.FSharp.Reflection
open System
open System.Configuration
open System.IO
open System.Runtime.Serialization
open RethinkDb
open RethinkDb.Configuration

[<CLIMutable>]
[<DataContract>]
type Product = {
    [<field: DataMember(Name="id") >]
    Id: string
}

[<EntryPoint>]
let main argv =
    let connFactory = ConfigurationAssembler.CreateConnectionFactory("example")
    let conn = connFactory.Get()
    let result = conn.Run(Query.Db("karltest").Table<Product>("products"))
    for product in result do
        printfn "%A" product
    0

This returns the following (there are 5 records in that table, all with valid Id properties):

{Id = null;}
{Id = null;}
{Id = null;}
{Id = null;}
{Id = null;}

It would be helpful if you could confirm if I am even using this properly in F# and, if not, if you could supply a better example of your working read-only code so we can get this working properly!

spiffytech commented 9 years ago

@karlgrz I'm using the JSON.NET serializer, since JSON.NET has good F# support. Here is some sample code I pulled from a few modules in my project:

open RethinkDb
open RethinkDb.Newtonsoft.Configuration

type Sleep = {
    // OptionConverter from http://gorodinski.com/blog/2013/01/05/json-dot-net-type-converters-for-f-option-list-tuple/
    // Allows None to be converted to null by JSON.NET
    [<JsonConverter(typeof<JsonConverters.OptionConverter>)>]
    // NullHandlingValue: don't emit value if value is null. Let RethinkDB generate an ID.
    [<JsonProperty(NullValueHandling = NullValueHandling.Ignore)>]
    id: int option;
    date: System.DateTime;
    typicalDay: bool;
    // Lots of other fields
  }

module Tables =
  let sleeps = Query.Db("sleep_log").Table<Sleep>("sleeps")

let connectionFactory = ConfigurationAssembler.CreateConnectionFactory("example");
let conn = connectionFactory.Get()

let printResponse (resp:DmlResponse) =
    printfn "Created: %i\nUpdated: %i\nReplaced: %i\nGenerated Keys: %A\n" (int resp.Inserted) (int resp.Updated) (int resp.Replaced) resp.GeneratedKeys

let storeSleep (sleep:Sleep) =
    let res = conn.Run(Tables.sleeps.Insert(sleep, Conflict.Replace))

    match res.GeneratedKeys with
    | [|newId|] -> Some newId
    | _ -> None

let getSleepsInRange start fin =
    conn.Run(
      Tables.sleeps.Between(start, fin, "date").OrderBy("date")
    )

let getSleepForDate (date:System.DateTime) =
    let bottomDate = date.Date
    let topDate = bottomDate.AddDays(1.)
    try
      conn.Run(Tables.sleeps.Between(bottomDate, topDate, "date").Nth(0))
      |> Some
    with
      | :? System.AggregateException as aex ->  // RethinkDB returns a wrapper for multiple exceptions
          match aex.InnerException with
          | :? RethinkDbRuntimeException -> None  // Assumed to be when the list is empty, since we requested .[0]
          | _ -> raise aex  // Unexpected other exceptions
karlgrz commented 9 years ago

@spiffytech this is great! Thanks! Hopefully I can get something you can try out soon.