FSharp.Data.SqlClient
FSharp.Data.SqlClient copied to clipboard
SqlProgrammabilityProvider requires open SqlConnection
Description
The CreateCommand
on SqlProgrammabilityProvider
is a great idea as it means that we just need a single type for the DB, whether for ad-hoc queries or inserts using datatables etc. However, the arguments required by the CreateCommand
method could be improved: -
Repro steps
This is the current way that you use the provider: -
type AdventureWorks = SqlProgrammabilityProvider<Conn>
let conn = new System.Data.SqlClient.SqlConnection(Conn)
conn.Open()
let customers = AdventureWorks.CreateCommand<"SELECT * FROM SalesLT.Customer">(conn).Execute() |> Seq.toArray
Expected behavior
This is what I would prefer to see: -
type AdventureWorks = SqlProgrammabilityProvider<Conn>
let customers = AdventureWorks.CreateCommand<"SELECT * FROM SalesLT.Customer">(Conn).Execute() |> Seq.toArray // use string rather than open SQL connection object
Or even better: -
type AdventureWorks = SqlProgrammabilityProvider<Conn>
let customers = AdventureWorks.CreateCommand<"SELECT * FROM SalesLT.Customer">().Execute() |> Seq.toArray // connection string is optional
Known workarounds
I suppose you could write an extension method or similar that takes in the command with a constraint of something like (pseudo code) (conn:SqlConnection) -> execute() -> Seq<'T>
but this is not ideal.
Hey Isaac, I don't think that's how it works. Here are the tests for CreateCommand, they don't require explicit runtime connection.
@dsevastianov I believe that's only valid if you use a config-based connection string. I'm using a literal connection string within a script, which requires a valid, already-opened SQL Connection. Ironically, using this provided method would be by far the quickest and easier way of getting to SQL data in a script - yet isn't possible because scripts don't play well with configuration files, and using literals means open sql client connections.
Ah, I see, this definitely looks like a bug.
Way to reproduce:
I'm not sure if this is a bug or "by design" - with other parts of the TP, there was a decision made to have different behaviour in terms of type generation depending on whether the connection string was sourced from config or via literals. Personally I would prefer to revert that decision.
I agree, I don't think it currently works as expected. In this snapshot I tried mixing Programmability declarations with config and literal connection. Once I try to call CreateCommand for literal case it assumes same signature for config case. This is somewhat convoluted example, but I think it demonstrates that making method signature dependent on particular type definition might be dangerous.
It seems to me it should work:
- with passing a connection (and will open it if needed, but won't dispose it in anycase)
- without passing a connnection (will create and dispose after done with statement)
If having it work without connection is a problem for codebase which want to avoid mistake of using design time connection, we can simply add a parameter to the type provider: PrecludeUsageOfDesignTimeConnection = true
and in this case, not generate those overloads.
@dmitry-a-morozov do you know if there was any progress on this - is this an F# blocker or something that can be done without a dependency on changes to F# 4.1?
@isaacabraham I have no idea. Luckily I'm back to F# but busy starting at new job. Sql Server is currently is not my priority but I see what I can do. Is it urgent?
Oh not critical. It was more that this was related to the discussion on Twitter regarding type providers the other day.
Having said that this is IMHO a real value add feature as it makes the provider even easier to use, especially from scripts. If one of us get a chance to look at this we will, but it won't be anytime this month.
I would like to see the requested overloads (connection string instead of open connection object). I'd like to use SqlProgrammabilityProvider
for everything, but until I can pass in a connection string, I'll stick to SqlCommandProvider
.
@smoothdeveloper I think this is still an issue that I would love to see resolved, ideally by making connection string completely optional or (at worst) replacing the connection object with a string.