Use nanodbc::describe_parameters()

[krlmlr]

instead of relying on SQLDescribeParams(). This feels like the more robust way -- to adapt parameter types to what the client sends rather than to what the server thinks it needs to receive.

Will require upgrading nanodbc. Upstream: https://github.com/nanodbc/nanodbc/pull/225. @jimhester: I can take a closer look if you think this is a good idea.

Related: #30.

I'm not sure if this is an issue with the Simba driver. The Microsoft driver (which is near unusable due to the "Invalid Descriptor Index" problem, #309) has this problem for local temporary tables. I have confirmed with printf debugging that this is caused by a failing SQLDescribeParams() call.

library(odbc)

db <- dbConnect(
  odbc(),
  dsn = "mssql-test-ms",
  uid = "kirill",
  pwd = keyring::key_get("mssql", "kirill")
)

sf <- data.frame(a = "1")
sf$SHAPE <- list(raw(10))

# Works with Microsoft drivers, doesn't work with FreeTDS
dbWriteTable(db, "##test", as.data.frame(sf))

# Doesn't work with either Microsoft or FreeTDS driver
dbWriteTable(db, "#test", as.data.frame(sf))
#> Error in result_insert_dataframe(rs@ptr, values, batch_rows): nanodbc/nanodbc.cpp:1617: 00000: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Implicit conversion from data type varchar to varbinary is not allowed. Use the CONVERT function to run this query.  [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Statement(s) could not be prepared.

# Works with Microsoft drivers, doesn't work with FreeTDS
dbWriteTable(db, "test", as.data.frame(sf))

dbRemoveTable(db, "test")

^{Created on 2020-06-10 by the reprex package (v0.3.0)}

CC @mloskot @detule.

[detule]

Hey @krlmlr

Jim already helped me backport this into nanodbc, and we tried to use it to enhance dbWriteTable similarly to what you suggest, see https://github.com/r-dbi/odbc/commit/7c4bc6ec56a4f632fbcfb5db6246077fd144f847

With it, I believe that the first and third in your examples should work with FreeTDS (there should be parity in terms of dbWriteTable between the Microsoft driver and FreeTDS).

There is a small wrinkle when writing to out-of-database tables which may be the case why your examples are failing uniformly: In order for the appropriate code-path to be taken, at least as currently coded, SQLColumns needs to return coherent results when describing the columns of the table that is the target of the dbWrite operation - in this case a table in temdb.

• With FreeTDS, this ODBC endpoint only works to describe columns in the current database - so if you are writing elsewhere we'll go back to the code-path where we are sending faux/varchar(255) column descriptions. I have a PR out in FreeTDS to make SQLColumns work out-of-current-catalog - if that gets pulled, hopefully you will see this resolved.
• When we patched odbc we made the code called to describe columns, odbcConnectionColumns an S4 method (that defaults to connection_sql_columns -> SQLColumns). So technically, to make FreeTDS work for you, one doesn't even need to wait until the driver is patched - you just need to write a method that dispatches for a "Microsoft SQL Server" connection, that calls the catalog specific sp_columns sproc (rather than the system-wide, this is the essence of the FreeTDS PR).

At any rate, for me with FreeTDS patched with the above PR, I get:

> sf <- data.frame(a = "1")
> sf$SHAPE <- list(raw(10))
> dbWriteTable(db, "##test", as.data.frame(sf))
> dbGetQuery(db, "SELECT * FROM tempdb.##test")
  a SHAPE
1 1
# Known failure, I believe has to do with indexing local temp tables
> dbWriteTable(db, "#test", as.data.frame(sf))
Error in result_insert_dataframe(rs@ptr, values, batch_rows) :
  nanodbc/nanodbc.cpp:1617: 00000: [FreeTDS][SQL Server]Invalid object name '#test'.  [FreeTDS][SQL Server]Statement(s) could not be prepared.
> dbWriteTable(db, "test", as.data.frame(sf))
> dbGetQuery(db, "SELECT * FROM test")
  a SHAPE
1 1
> db@info$drivername
[1] "libtdsodbc.so"

[jimhester]

@detule adding that S4 method to odbc for FreeTDS seems worthwhile, as I imagine many people will be using older drivers for a while. Would you be interested in doing a PR for it?

[hadley]

@detule is this issue still needed or are we done?