Fix copy_to() for MSSQL databases

[krlmlr]

A regression.

The partial NextMethod() calls didn't work for me, only when the full list of arguments is specified.

[hadley]

Can you construct a reprex that illustrates the basic problem? This seems correct to me:

foo <- function(x, y = 1, ...) UseMethod("foo")

foo.bar <- function(x, y = 2, ...) NextMethod(z = 2)
foo.baz <- function(x, y = 3, z = 3, ...) list(y = y, z = z, ...)

foo(structure(list(), class = c("bar", "baz")), y = 0)
#> $y
#> [1] 0
#> 
#> $z
#> [1] 2

^{Created on 2019-04-10 by the reprex package (v0.2.1.9000)}

[hadley]

Is there some reason you can no longer use NextMethod() here?

[krlmlr]

The syntax is different in MSSQL. Originally implemented in #50, f283e03 reverted to using the parent method.

https://stackoverflow.com/q/16683758/946850

Adding a comment to the code.

[krlmlr]

I have reduced the NextMethod() calls somewhat.

This isn't exactly what I've seen, but something weird is going on:

foo <- function(x, y = 1, ...) UseMethod("foo")

foo.bar <- function(x, y = 2, z = 2, ...) NextMethod(z = z + 1, b = 5)
foo.baz <- function(x, y = 3, z = 3, ...) moo(x, y, z, ...)

moo <- function(x, y, z, ...) UseMethod("moo")
moo.bar <- function(x, y, z, ...) NextMethod(y = 6)
moo.baz <- function(x, y, z, ...) list(y = y, z = z, ...)

foo(structure(list(), class = c("bar", "baz")), y = 2)
#> $y
#> [1] 6
#> 
#> $z
#> [1] 2
#> 
#> [[3]]
#> [1] 3
#> 
#> $b
#> [1] 5

^{Created on 2019-04-11 by the reprex package (v0.2.1.9000)}

[krlmlr]

For reference, this is what I see on a live database with krlmlr/dbplyr@390d23a. When debugging, the arguments are garbled:

library(tidyverse)
library(DBI)
library(dbplyr)

mssql_con <- function() {
  DBI::dbConnect(
    odbc::odbc(),
    Driver = "SQL Server Driver",
    ...,
    TDS_Version = 7.4
  )
}

con <- mssql_con()

dbExecute(con, "SET NOCOUNT ON")
#> [1] 0

src <- src_dbi(con)

iris_tbl <- copy_to(src, iris, name = "iris", overwrite = TRUE, temporary = FALSE)
#> Error in (function (classes, fdef, mtable) : unable to find an inherited method for function 'dbWriteTable' for signature '"Microsoft SQL Server", "SQL", "character"'

^{Created on 2019-04-11 by the reprex package (v0.2.1.9000)}

[hadley]

Hmmm, maybe it's some weird interaction with S4.

[hadley]

Could we test this without an activate MS SQL backend by mocking dbExecute() to just return the sql, and then using expect_known_output()?

[krlmlr]

Perhaps with an approach similar to dbi-log. But then we still need to double-check the generated SQL.

Would you like me to open a new issue to discuss?

[hadley]

No, I just want a simple way to test this code without adding additional complexity or new dependencies.

[krlmlr]

Filed an issue: #284. Suggesting to rely on user testing for this PR, discuss/implement #284 for the v1.4.0 release and add some tests too.

[krlmlr]

The reprex in #280 (comment) doesn't use S4.