fsprojects / FSharp.Data.SqlClient

A set of F# Type Providers for statically typed access to MS SQL database
http://fsprojects.github.io/FSharp.Data.SqlClient/
Other
204 stars 71 forks source link

SqlProgrammabilityProvider requires open SqlConnection #245

Open isaacabraham opened 8 years ago

isaacabraham commented 8 years ago

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.

dsevastianov commented 8 years ago

Hey Isaac, I don't think that's how it works. Here are the tests for CreateCommand, they don't require explicit runtime connection.

isaacabraham commented 8 years ago

@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.

dsevastianov commented 8 years ago

Ah, I see, this definitely looks like a bug.

dsevastianov commented 8 years ago

Way to reproduce:

image

isaacabraham commented 8 years ago

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.

dsevastianov commented 8 years ago

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.

smoothdeveloper commented 7 years ago

It seems to me it should work:

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.

isaacabraham commented 7 years ago

@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?

dmitry-a-morozov commented 7 years ago

@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?

isaacabraham commented 7 years ago

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.

cmeeren commented 6 years ago

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.

isaacabraham commented 5 years ago

@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.