Not Implemented Exception While Using SRTP
1eyewonder opened this issue · 6 comments
Issue
Calling code similar to below will result in a Not Implemented Exception. Area of code is found here for the 'isIn' function https://github.com/Dzoukr/Dapper.FSharp/blob/master/src/Dapper.FSharp/MSSQL/LinqExpressionVisitors.fs#L218
Use Case
I have a quite a few similar queries where I need to check for existence in various tables and other minute validations. If I am thinking about this correctly, I think adding support for SRTP would provide benefit in being able to create re-usable queries so I don't have to create a new query for each table.
#r "nuget: Dapper.FSharp"
open Dapper.FSharp
open Dapper.FSharp.MSSQL
open System.Data
[<CLIMutable>]
type Person =
{
Id : int
}
[<CLIMutable>]
type Dog =
{
Id : int
}
let people = table'<Person> "People"
let dogs = table'<Dog> "Dogs"
let inline getIds<'T
when 'T: (member Id : int)>
(ids : int list)
(querySource : QuerySource<'T>)
(conn : IDbConnection)
= async {
// This is required in order to use SRTP w/ the query builder
let getId (item : 'T) = item.Id
let query = select {
for row in querySource do
where (isIn (getId row) ids)
}
return! conn.SelectAsync<'T>(query) |> Async.AwaitTask
}
I'm pretty sure that only the SelectAsync<'T>
method reflects the columns to read.
So rather than using an SRTP, you could probably just create a single partial object that has only the Id
field for querying, and then pass in a generic argument of the full table class that contains all the properties you actually want to read.
[<CLIMutable>]
type IdColumn = { Id: int }
let queryByIds<'T> (ids: int list) tableName (conn: IDbConnection) =
async {
let query =
select {
for row in table'<IdColumn> tableName do
where (isIn row.Id ids)
}
return! conn.SelectAsync<'T>(query) |> Async.AwaitTask
}
let! dogs = queryByIds<Dog> [1;2;3] "Dogs" conn
Wow, I feel stupid for not thinking of this 🤦♂️. Thank you, this makes sense!
Not stupid at all! I’m just very familiar with the internals of the codebase.
So more of a philosophical question now. Is the SRTP support still a valid request/issue for this repo? While the 'ID' use case is very straight forward, does it make sense to suggest this route for other alternatives? Silly examples below.
[<CLIMutable>]
type IdColumn = { Id: int }
[<CLIMutable>]
type IdAndFoo1Column = { Id: int; Foo1: int; }
[<CLIMutable>]
type IdFoo1Foo2Column = { Id: int; Foo1: int; Foo2: int }
So while code quantity wouldn't really differ between the DTO and the SRTP routes, the pain of naming is placed on users(devs). The STRP would just allow users to focus on what is needed for the generic queries rather than come up with a DTO for each use case. (Granted I'm not sure how often users come across this since I haven't really dealt with it until the other day). I'm sure it may just end up being diminishing returns trying to pursue this route, but I figured I'd ask the question while the issue was still open. If you think it's not worth pursuing, I can go ahead and close the issue.
Personally, I will always opt for an extra type or function over SRTP for simplicity.
I think using an anonymous record would be the better play in this case:
let queryByIds<'T> (ids: int list) tableName (conn: IDbConnection) =
async {
let query =
select {
for row in table'<{| Id: int |}> tableName do
where (isIn row.Id ids)
}
return! conn.SelectAsync<'T>(query) |> Async.AwaitTask
}
let! dogs = queryByIds<Dog> [1;2;3] "Dogs" conn
Yep, that makes sense. Thank you so much for your help!