fsprojects / FSharp.Linq.ComposableQuery

Compositional Query Framework for F# Queries, based on "A Practical Theory of Language-Integrated Query"
http://fsprojects.github.io/FSharp.Linq.ComposableQuery/
MIT License
67 stars 13 forks source link

Destructuring not supported for quoted funs? #17

Open cmeeren opened 5 years ago

cmeeren commented 5 years ago

I am using this through SQLProvider. Let me know if this issue should be posted in that repo instead.

Consider the following:

type OrgId = OrgId of int

let isProjectOwnedBy =
  <@ fun (OrgId owner) (p: MyDbType.dataContext.``dbo.ProjectEntity``) ->
    p.OrgId = Some owner
  @>

let projectsOwnedBy (orgId: OrgId) =
  let ctx = MyDbType.GetDataContext ()
  query {
    for p in ctx.Dbo.Project do
    where ((%isProjectOwnedBy) orgId)
    select p
  }

This fails at runtime with the following error (I've modified it to fit the simplified example above, so there may be errors):

System.Exception: 'Unsupported expression. Ensure all server-side objects won't have any .NET-operators/methods that can't be converted to SQL. The In and Not In operators only support the inline array syntax. (owner => (tupledArg.Item5.GetColumnOption("OrgId") == Some(owner)).Invoke(value(System.Runtime.CompilerServices.StrongBox`1[OrgId]).Value.Item))'

However, it works if I don't destructure in the quoted function:

type OrgId = OrgId of int with
  member this.Wrapped = let (OrgId x) = this in x

let isProjectOwnedBy =
  <@ fun (owner: OrgId) (p: MyDbType.dataContext.``dbo.ProjectEntity``) ->
    p.OrgId = Some owner.Wrapped
  @>

Is this intended? Am I doing things wrong?

jamescheney commented 5 years ago

Thanks, this is an interesting example.

Generally, FSharpComposableQuery is based on a normalization strategy that is complete for a small subset of F# query expressions and other features only - roughly speaking, functions over arguments built up out of base types, records, or sequences, and "simple" conjunctive queries only. Outside of this subset, it makes a best effort but is not guaranteed to successfully normalize queries, so you may get an error, or a query avalanche where a single query expression generates a loop with a query inside it (i.e. N+1 query problem).
I don't think either case is something we have explicitly considered. In the first example, I think the error you are seeing is arising because:

(It is also worth mentioning that SQLProvider didn't exist or at least was much less mature when we first developed this, and hasn't been tested thoroughly - see #15 for example - but this doesn't seem to be the main obstacle here.)

The second example seems to somehow circumvent this, by defining and calling a method, which appears to lead to the non-SQL-izable constructs being normalized away successfully. But I'm actually somewhat surprised this worked - it is probably because the underlying LINQ library support in SQLProvider is able to deal with this even though FSharpComposableQuery does not.

One way to test this is to manually inline the two different versions of the function and try running the query without FSHarpComposableQuery, to see if different ways of unpacking the OrgId integer work or don't, since I think the main thing FSharpComposableQuery will be doing in this case is the inlining/simplification.

In this example, what I would suggest is to write isProjectOwnedBy as a function taking an integer rather than orgId, and do the pattern matching/unpacking of orgId outside of the query / quoted expressions, so that it definitely happens on the F# side.

I think extending to support variant types, or objects, more systematically would be a good extension but at this point I don't really have the resources to maintain/extend this library. (Though if there were evidence that this woule benefit people I might be able to find some.)

jamescheney commented 5 years ago

This might be a good example to illustrate the current limitations of FSharpComposableQuery, would it be OK to adapt it for use in the documentation somewhere?

cmeeren commented 5 years ago

Yes, please use and adapt as desired.

As a side note, I posted https://github.com/fsprojects/SQLProvider/issues/615 in SQLProvider, but it might belong here - could you take a quick look and let me know if I should post it here instead?

jamescheney commented 5 years ago

Thanks. I had a look at the SQLProvider issue you mentioned. I can't tell from it whether you ran the example with just SQLProvider or with FSharpComposableQuery. If you've tried both, was there a difference? If you get the same behavior either way then my guess is the problem is with SQLProvider. If you get correct behavior with SQLProvider but wrong behavior if you are in addition using FSharpComposableQuery then I guess it is not an SQLProvider issue, but I can't off the top of my head think of a reason why this would happen.

As with the problem above, it also might be helpful to try manually inlining or simplifying the quoted function in different ways too, i.e. see if you already get incorrect behavior from SQLProvider with

query {
    for c, u in (for c in ctx.Dbo.Card do
    for u in ctx.Dbo.User do
    select (c, u)) do
    select (c.Id, u.Id)
  }

or

query {
    for c in ctx.Dbo.Card do
    for u in ctx.Dbo.User do
    select (c.Id, u.Id)
  }

(Incidentally, there is also a rather old issue there fsprojects/SQLProvider#48 about whether FSharpComposableQuery works with SQLProvider...)

cmeeren commented 5 years ago

I can't tell from it whether you ran the example with just SQLProvider or with FSharpComposableQuery.

Is there a difference? AFAIK SQLProvider integrates FSharpComposableQuery:

Generate composable queries by using FSharp.Linq.ComposableQuery

The SQLProvider also supports composable queries by integrating following library FSharpLinqComposableQuery. You can read more about that library here: FSharp.Linq.ComposableQuery

Because it is implemented in the SQLProvider you dont need to add FSharpComposableQuery in your script.


Thanks for the inlining tip. The problem persists when inlining. I have updated the other issue.

jamescheney commented 5 years ago

Ah, OK. I remember they was some discussion of doing this but wasn't sure what the status was.

If they've incorporated our code (rather than just making this library a dependency) then I'm happy to help with tracking down any problems with it in SQLProvider but the issue belongs there.

I should discuss with them how to explain the relationship and handle issues if FSharpComposableQuery code is incorporated into SQLProvider.