FSharp.Data.SqlClient icon indicating copy to clipboard operation
FSharp.Data.SqlClient copied to clipboard

Allow use pre-existing type for SqlCommandProvider

Open VTJDailey opened this issue 6 years ago • 3 comments

I would find it useful to have a new ResultType option for specifying a user-supplied type as the output type for a query. This may be very vaguely related to #326.

In the scenario where I would have use for this, I've got a set of 7 or 8 related search queries that each search on different criteria, but all return a result set with the same shape. It's also a fairly wide result set, and I end up with a couple hundred lines of duplicated code parsing and mapping each command's Execute result into a domain type.

VTJDailey avatar Apr 10 '19 13:04 VTJDailey

Thanks for the suggestion, I'm actually looking forward to remove/obsolete ResultType all together and move toward a separated "prepare", and "execute"

The advantage of this:

  • "prepare" can happen without a hot context
  • "execute" can be decorrelated from result type by exposing several methods (ExecuteReader, ExecuteDataTable, ExecuteRecords) and those can also take hot context overrides
  • extension methods should be able to be defined in a generic manner without rellying on SRTP

Adding a member to ResultType would be a step in opposite direction.

To come back to your issue, if you are trying to unify several commands around a single DataTable or Record result set, here is what my current approach looks like:

  • do an unsafe unbox with plenty of comments surrounding (it just works so long the result set is the same)
  • add some lint check / unit test using sp_describe_first_result_set that verifies the column types/names are consistent among items for every sets I've defined

for sets of queries that define similar but not identical resultset, I wrap in a custom DU, and expose members that are needed in my logic in an unified fashion, for now, it looks a bit verbose, even using active pattern, but it is just declarative code and remains fully type checked and using total functions.

Could you please show us a sample to better understand what your suggestion would look like?

Thanks.

smoothdeveloper avatar Apr 10 '19 13:04 smoothdeveloper

Hi, here's a somewhat contrived example. There's two ways to do some search that have the same select clause, but will have different sets of JOINs and WHERE clause in order to search by different things.

type SearchResult = {
  FirstName: string option;
  LastName: string;
  AccountNumber: string;
  .........
}

type searchByAccountCmd = SqlCommandProvider<byAccountSQL, "name=xxx", ResultType=ResultType.UserProvidedType, ProvidedType=typeof<SearchResult>>

type searchByNameCmd = SqlCommandProvider<byNameSQL, "name=xxx", ResultType=ResultType.UserProvidedType, ProvidedType=typeof<SearchResult>>

let searchResults = 
  match searchParameters with
  | SearchType.ByAccount accountNumber ->
      use cmd = new searchByAccountCmd(connection)
      cmd.Execute(AccountNumber = accountNumber)
  | SearchType.ByName searchText ->
      let searchTerms = tokenizeTextAndCreateTVP searchText
      use cmd = new searchByNameCmd(connection)
      cmd.Execute(SearchTerms = searchTerms)
  |> Seq.map mapSearchResultToBusinessObject

Essentially, the gain I'm looking for is that there is a single 'mapSearchResultToBusinessObject' function required and every branch of the match statement can feed straight into it.

As it is currently I have a situation with about seven different search types and despite each SELECT clause being identical they each require their own individual mapping function to merge into a common 'SearchResult'. Given that it has a pretty wide result set, multiplied by seven different commands, I've got a couple hundred lines of code that is just the same thing repeated over and over again.

It's not the end of the world, honestly, but seems like a nice small improvement. I would expect that at compile time I would compare the SQL result set to the provided type and ensure all field names and types correspond.

I would say that 'do an unsafe unbox' is not the direction I would prefer to go with.

Do you have a write-up somewhere of what the current thinking is on the Prepare/Execute potential change? Obviously I could see an ExecuteRecords<T> generic function used to provide similar functionality but we would lose the possibility of doing a compile time verification that the record and SQL resultset fields match up.

VTJDailey avatar Apr 10 '19 19:04 VTJDailey

Thanks @VTJDailey, I'll look more into your comment and see if I have any idea with current state of the code.

For your code sample, we'd depend on https://github.com/fsharp/fslang-suggestions/issues/212 :

type GreatType = GreatTypeProvider<ProvidedType=typeof<SearchResult>>

With current F# compiler, we can only pass literals to TP, we'd need another idea for the API as of now.

I could see an ExecuteRecords generic function used to provide similar functionality but we would lose the possibility of doing a compile time verification

with that issue 212 above, we could gain that possibility 🙂

smoothdeveloper avatar Apr 10 '19 20:04 smoothdeveloper