odbc
odbc copied to clipboard
Use nanodbc::describe_parameters()
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.
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 makeSQLColumns
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 toconnection_sql_columns
->SQLColumns
). So technically, to makeFreeTDS
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 specificsp_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"
@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?
@detule is this issue still needed or are we done?