diff --git a/DESCRIPTION b/DESCRIPTION index fe04b85..b70e123 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -40,7 +40,8 @@ Suggests: knitr, rmarkdown, withr, - testthat (>= 3.0.0) + testthat (>= 3.0.0), + dbMatrix VignetteBuilder: knitr Config/testthat/edition: 3 Collate: diff --git a/R/connection-methods.R b/R/connection-methods.R index 0f516a5..2761079 100644 --- a/R/connection-methods.R +++ b/R/connection-methods.R @@ -71,6 +71,11 @@ if (is.null(dbdir) || dbdir == "" || dbdir == ":memory:") { return(tryCatch( { + warning( + "In-memory DuckDB reconnection creates a fresh empty database; ", + "prior tables and session-local state cannot be restored.", + call. = FALSE + ) new_con <- .connect_duckdb_lock_safe(dbdir = ":memory:") new_con }, @@ -81,12 +86,11 @@ # File-based reconnection tryCatch( { + dbdir <- .norm_path(dbdir) if (!file.exists(dbdir)) { return(NULL) } - new_con <- .connect_duckdb_lock_safe(dbdir = dbdir) - .reg_set_conn(dbdir, new_con) - new_con + .reg_get_or_connect(dbdir) }, error = function(e) NULL ) @@ -131,13 +135,6 @@ return(.db_recon(con, dir)) } - # Check registry for an existing valid connection to this db - cached <- .reg_conn(dir) - if (!is.null(cached)) { - return(cached) - } - - # Fallback to direct reconnection .db_recon(con, dir) }, error = function(e) { diff --git a/R/connection-registry.R b/R/connection-registry.R index 3928913..ef03ae0 100644 --- a/R/connection-registry.R +++ b/R/connection-registry.R @@ -10,34 +10,46 @@ #' @keywords internal .db_registry <- new.env(parent = emptyenv()) -#' Find a cached live connection for a database path -#' -#' @param dir Normalized path to database file -#' @return A valid DBIConnection or NULL -#' @keywords internal -#' @noRd -.reg_conn <- function(dir) { - if (is.null(dir) || dir == "" || dir == ":memory:") return(NULL) - dir <- .norm_path(dir) - key <- paste0("conn:", dir) +.reg_key <- function(dbdir) { + if (is.null(dbdir) || length(dbdir) != 1 || is.na(dbdir) || + dbdir == "" || dbdir == ":memory:") { + return(NULL) + } + paste0("conn:", .norm_path(dbdir)) +} + +.reg_conn <- function(dbdir) { + key <- .reg_key(dbdir) + if (is.null(key)) return(NULL) + cached <- .db_registry[[key]] if (!is.null(cached) && DBI::dbIsValid(cached)) return(cached) + if (!is.null(cached)) rm(list = key, envir = .db_registry) NULL } -#' Store a live connection in the registry -#' -#' @param dir Normalized path to database file -#' @param conn A valid DBIConnection -#' @keywords internal -#' @noRd -.reg_set_conn <- function(dir, conn) { - if (is.null(dir) || dir == "" || dir == ":memory:") return(invisible(FALSE)) - dir <- .norm_path(dir) - .db_registry[[paste0("conn:", dir)]] <- conn +.reg_set_conn <- function(dbdir, conn) { + key <- .reg_key(dbdir) + if (is.null(key) || is.null(conn) || !DBI::dbIsValid(conn)) { + return(invisible(FALSE)) + } + .db_registry[[key]] <- conn invisible(TRUE) } +.reg_get_or_connect <- function(dbdir) { + cached <- .reg_conn(dbdir) + if (!is.null(cached)) return(cached) + + if (is.null(.reg_key(dbdir))) { + return(.connect_duckdb_lock_safe(dbdir = ":memory:")) + } + + con <- .connect_duckdb_lock_safe(dbdir = .norm_path(dbdir)) + .reg_set_conn(dbdir, con) + con +} + #' Reset the dbProject registry #' #' @description Removes all entries from the dbProject registry diff --git a/R/dbMatrix-pin.R b/R/dbMatrix-pin.R index 1ce5226..0efc82d 100644 --- a/R/dbMatrix-pin.R +++ b/R/dbMatrix-pin.R @@ -297,7 +297,10 @@ read_pin_conn.conn_matrix_table <- function(x) { if (is.null(dbdir) || is.na(dbdir)) { cli::cli_abort("No database path found in pinned object metadata.") } - con <- .connect_duckdb_lock_safe(dbdir = dbdir) + if (!file.exists(.norm_path(dbdir))) { + cli::cli_abort("Database file {.path {dbdir}} not found for pinned object.") + } + con <- .reg_get_or_connect(dbdir) table_name <- x$table_name if (is.null(table_name) || is.na(table_name)) { diff --git a/R/dbProject-R6.R b/R/dbProject-R6.R index 9676553..ae1ef4c 100644 --- a/R/dbProject-R6.R +++ b/R/dbProject-R6.R @@ -47,6 +47,7 @@ dbProject <- R6::R6Class( drv = duckdb::duckdb() ) } + private$conn_ <- private$register_connection(private$conn_) connections::connection_pin_write( board = private$board, x = private$conn_, @@ -83,6 +84,7 @@ dbProject <- R6::R6Class( board = private$board, name = "cachedConnection" ) + private$conn_ <- private$register_connection(private$conn_) } invisible(self) }, @@ -113,6 +115,7 @@ dbProject <- R6::R6Class( dbdir = .(dbdir_val) ) )) + private$conn_ <- private$register_connection(private$conn_) connections::connection_pin_write( board = private$board, @@ -136,6 +139,7 @@ dbProject <- R6::R6Class( stop("No active or cached connection available") } } + private$conn_ <- private$register_connection(private$conn_) private$conn_@con }, @@ -302,6 +306,7 @@ dbProject <- R6::R6Class( board = private$board, name = "cachedConnection" ) + private$conn_ <- private$register_connection(private$conn_) } else { # Add to return list for user to assign restored[[pin_name]] <- self$pin_read(pin_name) @@ -458,6 +463,25 @@ dbProject <- R6::R6Class( has_cached_connection = function() { "cachedConnection" %in% pins::pin_list(private$board) + }, + + register_connection = function(conn_obj) { + if (is.null(conn_obj) || is.null(conn_obj@con)) { + return(conn_obj) + } + + dbdir <- tryCatch(.get_dbdir(conn_obj@con), error = function(e) NULL) + cached <- .reg_conn(dbdir) + if (!is.null(cached)) { + if (!identical(cached, conn_obj@con)) { + try(DBI::dbDisconnect(conn_obj@con), silent = TRUE) + conn_obj@con <- cached + } + } else { + .reg_set_conn(dbdir, conn_obj@con) + } + + conn_obj } ) ) diff --git a/R/dbSpatial-pin.R b/R/dbSpatial-pin.R index 9224f55..e0cf655 100644 --- a/R/dbSpatial-pin.R +++ b/R/dbSpatial-pin.R @@ -67,8 +67,11 @@ read_pin_conn.conn_spatial_table <- function(x) { if (is.null(dbdir) || is.na(dbdir)) { cli::cli_abort("No database path found in pinned object metadata.") } + if (!file.exists(.norm_path(dbdir))) { + cli::cli_abort("Database file {.path {dbdir}} not found for pinned object.") + } - con <- .connect_duckdb_lock_safe(dbdir = dbdir) + con <- .reg_get_or_connect(dbdir) table_name <- x$table_name if (is.null(table_name) || is.na(table_name)) { diff --git a/tests/testthat/test-dbData-reconnect.R b/tests/testthat/test-dbData-reconnect.R index 41bd125..15339f2 100644 --- a/tests/testthat/test-dbData-reconnect.R +++ b/tests/testthat/test-dbData-reconnect.R @@ -54,3 +54,30 @@ test_that("dbReconnect returns object with reconnected tbl", { DBI::dbDisconnect(refreshed_con) }) + +test_that("dbReconnect reuses one file-backed connection", { + dbProject:::.reg_reset() + withr::defer(dbProject:::.reg_reset()) + + db_path <- tempfile(fileext = ".duckdb") + con <- DBI::dbConnect(duckdb::duckdb(), dbdir = db_path) + DBI::dbExecute(con, "CREATE TABLE one AS SELECT 1 AS x") + DBI::dbExecute(con, "CREATE TABLE two AS SELECT 2 AS x") + + if (!methods::isClass("RegistryDbData")) { + methods::setClass("RegistryDbData", contains = "dbData", where = .GlobalEnv) + } + one <- methods::new("RegistryDbData", value = dplyr::tbl(con, "one")) + two <- methods::new("RegistryDbData", value = dplyr::tbl(con, "two")) + + DBI::dbDisconnect(con, shutdown = TRUE) + one <- dbReconnect(one) + two <- dbReconnect(two) + + con1 <- dbplyr::remote_con(one@value) + con2 <- dbplyr::remote_con(two@value) + expect_identical(con2, con1) + expect_equal(dplyr::collect(two@value)$x, 2) + + DBI::dbDisconnect(con1) +})