SQL Server integration

[simonmcconnell]

Hi,

I've added SQL Server as an option in a fork. Before I submit the PR, I have a question regarding options. It seems the status quo is to provide relatively few options. I think for SQL Server, adding instance would be the bare minimum, but my instinct was to add a few more. Which way do you prefer to lean, i.e. would you want ssl and/or the bottom row in or out?

[josevalim]

Hi @simonmcconnell! Let's go lean. You can always convert the smart cell to code if you need additional options. :)

[simonmcconnell]

Sure thing. I'm not sure if it is viable to implement the SQL query cell. Tds requires query parameters to be given a name, e.g. @SomeName. It doesn't seem to support the ? method of parameter substitution.

Given that SQLCell.to_quoted can only currently name a parameter based on the index, users would have to ensure they name each parameter @1, @2, etc in order they're used in the SQL Cell.

p1 = %Tds.Parameter{value: 1018, name: "@1"}
p2 = %Tds.Parameter{value: 1019, name: "@2"}

SELECT tagid, timestamputc, tagvalue FROM sometable WHERE tagid IN ({{p1}}, {{p2}})

all to_quoted gets as attrs is

sql_cell.to_quoted.attrs: %{
  "cache_query" => true,
  "connection" => %{"type" => "sqlserver", "variable" => "conn"},
  "query" => "SELECT tagid, timestamputc, tagvalue\n  FROM sometable\n  WHERE tagid IN ({{p1}}, {{p2}}",
  "result_variable" => "result",
  "timeout" => nil
}

and I don't see how I could get the name from the Parameter down in parameterize.

[josevalim]

Could you assign random names (@p1, @p2, etc). This would be similar to PostgreSQL, which has to use $1, $2, etc. WDYT?

[jonatanklosko]

@simonmcconnell is there a reason that the user should define the parameters explicitly? The idea with SQL cell is to interpolate regular variables. Ideally if they have number = 1018, then in the query they do {{number}}, we could replace it with @1 and automatically build %Tds.Parameter{value: number, name: "@1"} in the parameter list.

[simonmcconnell]

That could work. I didn't consider that.

I haven't touched Tds for a few years before this week, but as far as I understand, you must pass a list of %Tds.Parameter{}s to Tds.query, so I was running with that. It might be confusing if the SQL cell takes plain old values when the library does not.

[jonatanklosko]

It might be confusing if the SQL cell takes plain old values

It's fine, the user can see the underlying code, and the point of SQL cell is that the user doesn't have to know specifics of the particular database driver :)

[simonmcconnell]

I'll give that a go. Thanks for the feedback lads.