feat: dm_from_con() gains .names argument for pattern-based construction of table names in the dm object

R/db-helpers.R

@@ -118,17 +118,37 @@ repair_table_names_for_db <- function(table_names, temporary, con, schema = NULL
  quote_ids(names, con_from_src_or_con(con), schema)
 }
 
-make_local_names <- function(schema_names, table_names) {
+find_name_clashes <- function(old, new) {
 
- combined_names <- glue::glue("{schema_names}.{table_names}")
+ # Any entries in `new` with more than one corresponding entry in `old`
+ purrr::keep(split(old, new), ~ length(unique(.x)) > 1)
+}
+
+make_local_names <- function(schema_names, table_names, repair = "minimal") {
 
- if (length(unique(schema_names)) == 1)
+ local_names <- if (length(unique(schema_names)) == 1)
  table_names
  else
- combined_names
+ glue::glue("{schema_names}.{table_names}")
+
+ repaired_local_names <- vctrs::vec_as_names(local_names, repair = repair)
+
+ # Ensure new table names don't clash
+ clashes <- find_name_clashes(local_names, repaired_local_names)
+
+ if (length(clashes) > 0)
+ cli::cli_abort(c(
+ "x" = "Repairing names caused name clashes; try again with a different `.name_repair` option.",
+ purrr::imap_chr(
+ clashes,
+ ~ glue::glue("* [{paste0(dQuote(.x, q = FALSE), collapse = ', ')}] => {dQuote(.y, q = FALSE)}")
+ )
+ ))
+
+ repaired_local_names
 }
 
-get_src_tbl_names <- function(src, schema = NULL, dbname = NULL) {
+get_src_tbl_names <- function(src, schema = NULL, dbname = NULL, .name_repair = "check_unique") {
  if (!is_mssql(src) && !is_postgres(src) && !is_mariadb(src)) {
  warn_if_arg_not(schema, only_on = c("MSSQL", "Postgres", "MariaDB"))
  warn_if_arg_not(dbname, only_on = "MSSQL")
@@ -164,7 +184,7 @@ get_src_tbl_names <- function(src, schema = NULL, dbname = NULL) {
  collect() %>%
  # create remote names for the tables in the given schema (name is table_name; cannot be duplicated within a single schema)
  transmute(
- local_name = make_local_names(schema_name, table_name),
+ local_name = make_local_names(schema_name, table_name, repair = .name_repair),
  remote_name = schema_if(schema_name, table_name, con, dbname)
  ) %>%
  deframe()

R/dm_from_con.R

@@ -16,12 +16,16 @@
 #' foreign keys from the database.
 #' Currently works only for Postgres and SQL Server databases.
 #' The default attempts to query and issues an informative message.
+#' @param .name_repair Treatment of problematic table names. This argument is
+#' passed on as `repair` to [vctrs::vec_as_names()].
 #' @param ... `r lifecycle::badge("experimental")`
 #'
 #' Additional parameters for the schema learning query.
 #'
 #' - `schema`: supported for MSSQL (default: `"dbo"`), Postgres (default: `"public"`), and MariaDB/MySQL
 #' (default: current database). Learn the tables in a specific schema (or database for MariaDB/MySQL).
+#' If multiple schema names are given as a vector, the returned `dm` object will have names of the form
+#' `"schema.table"`.
 #' - `dbname`: supported for MSSQL. Access different databases on the connected MSSQL-server;
 #' default: active database.
 #' - `table_type`: supported for Postgres (default: `"BASE TABLE"`). Specify the table type. Options are:
@@ -40,7 +44,7 @@
 #'
 #' # Avoid DBI::dbDisconnect() here, because we don't own the connection
 dm_from_con <- function(con = NULL, table_names = NULL, learn_keys = NULL,
- ...) {
+ .name_repair = "check_unique", ...) {
  stopifnot(is(con, "DBIConnection") || inherits(con, "Pool"))
 
  if (inherits(con, "Pool")) {
@@ -84,7 +88,7 @@ dm_from_con <- function(con = NULL, table_names = NULL, learn_keys = NULL,
  }
 
  if (is_null(table_names)) {
- src_tbl_names <- get_src_tbl_names(src, ...)
+ src_tbl_names <- get_src_tbl_names(src, ..., .name_repair = .name_repair)
  } else {
  src_tbl_names <- table_names
  }

tests/testthat/test-db-helpers.R

@@ -176,4 +176,55 @@ test_that("make local names", {
 
  c("sch1.tbl1", "sch2.tbl2", "sch3.tbl3")
  )
+
+
+ # If we have name clashes, there should be an error about non-unique names
+ expect_equal(
+ make_local_names(
+ rep("sch", 2),
+ c("table name", "Table Name"),
+ repair = "check_unique" # default from dm_from_con()
+ ),
+ c("table name", "Table Name") # i.e. these names are unique & therefore fine
+ )
+
+ expect_snapshot_error(
+ make_local_names(
+ rep("sch", 2),
+ c("table name", "Table Name"),
+ repair = tolower # passing in a custom "repair" function which leads to a clash
+ )
+ )
+
+ expect_equal(
+ make_local_names(
+ c("sch1", "sch2"), # this time, tables are in different schemas - so no clash of "schema.table" local names
+ c("table name", "Table Name"),
+ repair = tolower
+ ),
+ c("sch1.table name", "sch2.table name")
+ )
+
+})
+
+test_that("find name clashes", {
+
+ # If all old names change to different new names...
+ res <- find_name_clashes(
+ c("one", "two", "three"),
+ c("uno", "dos", "tres")
+ )
+ # ... we shouldn't get anything
+ expect_length(res, 0)
+
+
+ # If multiple old names change to the same new name...
+ res <- find_name_clashes(
+ c("one", "two", "three"),
+ c("uno", "uno", "tres")
+ )
+ # We should get a list, with one element per "clashing" new name
+ expect_named(res, "uno")
+ expect_equal(res[["uno"]], c("one", "two"))
+
 })