Error: Invalid connection with dbplyr:::with_transaction()

psimm commented 3 years ago

Brief description of the problem

I'm testing a query that uses dplyr::left_join(copy = TRUE). When I mock the query, I get the error "Invalid connection" which is raised by RPostgres:::connection_is_transacting(conn@ptr).

This issue is similar to #144. I'm using the latest dittodb version from Github. The problem here is related to the copy argument which then causes dbplyr to start a transaction. Perhaps a method for handling it is missing?

The kind of database backend you are trying to test

PostgresSQL 13.3 with package RPostgres

Reprex

library(DBI)
library(dittodb)
library(dplyr)
library(testthat)

db_connect <- function() {
  dbConnect(
    drv = RPostgres::Postgres(),
    host = "host",
    user = "user",
    password = "password",
    dbname = "dbname"
  )
}

dbWriteTable(
  conn = db_connect(),
  name = "test",
  value = data.frame(label = c("a", "a", "b", "b"), value = c(1, 2, 3, 4))
)

myfun <- function(conn) {
  local_df <- data.frame(label = "a", value2 = 5)
  tbl(conn, "test") %>% 
    left_join(local_df, by = "label", copy = TRUE) %>% 
    collect()
}

start_db_capturing()
conn <- db_connect()
act <- myfun(conn)
dbDisconnect(conn)
stop_db_capturing()

with_mock_db({
  test_that("myfun works", {
    conn <- db_connect()
    act <- myfun(conn)
    dbDisconnect(conn)
    expect_true(is.data.frame(act))
  })
})

Backtrace:

── Error (Line 4): myfun works ─────────────────────────────────────────────────
Error: Invalid connection
Backtrace:
  1. global::myfun(conn)
  5. dbplyr:::left_join.tbl_lazy(., local_df, by = "label", copy = TRUE)
  6. dbplyr:::add_op_join(...)
  8. dbplyr:::auto_copy.tbl_sql(x, y, copy = copy, indexes = if (auto_index) list(by$y))
 10. dbplyr:::copy_to.src_sql(...)
 12. dbplyr:::db_copy_to.DBIConnection(...)
 13. dbplyr:::with_transaction(...)
 15. RPostgres::dbBegin(con)
 16. RPostgres:::connection_is_transacting(conn@ptr)

sessionInfo()

R version 4.1.0 (2021-05-18)
Platform: x86_64-apple-darwin17.0 (64-bit)
Running under: macOS Big Sur 11.5.2

Matrix products: default
LAPACK: /Library/Frameworks/R.framework/Versions/4.1/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices datasets  utils     methods   base     

other attached packages:
[1] testthat_3.0.4     dplyr_1.0.7        dittodb_0.1.3.9000 DBI_1.1.1         

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.7        pillar_1.6.2      compiler_4.1.0    dbplyr_2.1.1      RPostgres_1.3.3  
 [6] tools_4.1.0       digest_0.6.27     bit_4.0.4         pkgload_1.2.1     lubridate_1.7.10 
[11] RSQLite_2.2.8     memoise_2.0.0     lifecycle_1.0.0   tibble_3.1.4      pkgconfig_2.0.3  
[16] rlang_0.4.11      cli_3.0.1         rstudioapi_0.13   fastmap_1.1.0     withr_2.4.2      
[21] generics_0.1.0    desc_1.3.0        vctrs_0.3.8       hms_1.1.0         bit64_4.0.5      
[26] rprojroot_2.0.2   tidyselect_1.1.1  glue_1.4.2        R6_2.5.1          fansi_0.5.0      
[31] sessioninfo_1.1.1 purrr_0.3.4       blob_1.2.2        magrittr_2.0.1    ellipsis_0.3.2   
[36] assertthat_0.2.1  renv_0.14.0       utf8_1.2.2        cachem_1.0.6      crayon_1.4.1

jonkeane commented 3 years ago

Thanks for another great report! You're right that this is some more missing methods, though it turns out that this is a bit more complicated than the first one. I have a branch with_transaction that is mostly working for RPostgres (though there is at least one really bad hack that needs to be addressed before I'll be able to merge it), (second) though it doesn't quite work with RMariaDB, so I need to investigate more there.

You might try using that and seeing if it resolves your issue (please let me know either way, so I know if I'm on the right track). If you run into another issue after this one, feel free to submit an issue now too — I might be able to fold them into the same PR

psimm commented 3 years ago

That works with the reprex and with the query I was originally working on. Awesome!

I'll use the branch to write my tests and update once you merge the PR. The query I was working with takes about 5 minutes to run and now thanks to dittodb it can be tested in a second - massive improvement! Thanks.

I opened another issue here: https://github.com/ropensci/dittodb/issues/150

ropensci / dittodb