tomroh / bcputility

R package for fast bulk imports/exports from/to SQL Server with the bcp command line utility
https://bcputility.roh.engineering
Other
14 stars 2 forks source link

Allow broader specification of TrustedConnection #13

Closed mkoohafkan closed 2 years ago

mkoohafkan commented 2 years ago

The definition of trustedconnection varies by driver. for example, the "Microsoft SQL Server Native Client" driver uses Trusted_Connection=Yes while the older "SQL Server" driver uses Trusted_Connection=True. Currently, it is not possible to use bcpImport with the Microsoft SQL Server Native Client driver because it cannot properly resolve the Trusted_Connection string (probably because the DBI/odbc methods assume the string should be "True", like the old SQL Server driver).

It would be great if the user could provide their own value of trustedconnection (rather than TRUE/FALSE) to work with other drivers. You could instead check isFALSE(trustedconnection) to determine if the username/password must be provided to bcp.

tomroh commented 2 years ago

I'm guessing the authentication fails with the DBI::dbConnect function call and it's default interpretation of trusted_connection. bcp shouldn't have this issue since it is only specified by -T. Can you provide a working example for both cases for DBI::dbConnect?

mkoohafkan commented 2 years ago

Yep, although further testing reveals both drivers accept the same values of trusted_connection (including boolean TRUE) so it's just the default behavior that is different. Therefore, allowing a character value for trustedconnection and explicitly passing the value to your dbConnect() call at https://github.com/tomroh/bcputility/blob/6fb9764e5ebb353c2f82a2c3bccff7d777764c62/R/bcp.R#L139-L142 should fix the issue.

con <- DBI::dbConnect(odbc::odbc(), 
    driver = driver,
    server = server, 
    database = database,
    trusted_connection = trustedconnection)

See https://github.com/r-dbi/odbc/issues/483 for example calls to dbConnect().

You could still use !isFALSE(trusted connection) instead to keep your branching logic for defining the connection object. Should allow "yes", "true", or TRUE to be passed and work with "SQL Server" driver.

mkoohafkan commented 2 years ago

Nope, I was right the first time. "SQL Server" can accept "true", "yes", and TRUE but Microsoft SQL Server Native Client only accepts "yes".

tomroh commented 2 years ago

285fa7ebd31d5b7fdaaa1b08286aeb73855398e2

This makes more sense from a Microsoft design standpoint that there would be compatibility. I'm setting the default to be "yes" since it works for both.

devtools::install_github('tomroh/bcputility@devel