fix: pass naming pattern through to dm_learn_from_db() (#2213)

cynkra · Aug 14, 2024 · d34dc70 · d34dc70
1 parent ff024e2
commit d34dc70
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 16 deletions.
diff --git a/R/db-helpers.R b/R/db-helpers.R
@@ -121,7 +121,7 @@ find_name_clashes <- function(old, new) {
 }
 
 #' @autoglobal
-get_src_tbl_names <- function(src, schema = NULL, dbname = NULL, names = NULL) {
+get_src_tbl_names <- function(src, schema = NULL, dbname = NULL, names_pattern = "{.table}") {
   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")
@@ -155,16 +155,6 @@ get_src_tbl_names <- function(src, schema = NULL, dbname = NULL, names = NULL) {
     names_table <- get_names_table_mariadb(con)
   }
 
-  # Use smart default for `.names`, if it wasn't provided
-  if (!is.null(names)) {
-    names_pattern <- names
-  } else if (length(schema) == 1) {
-    names_pattern <- "{.table}"
-  } else {
-    names_pattern <- "{.schema}.{.table}"
-    cli::cli_inform('Using {.code .names = "{names_pattern}"}')
-  }
-
   names_table <- names_table %>%
     filter(schema_name %in% !!(if (inherits(schema, "sql")) glue_sql_collapse(schema) else schema)) %>%
     collect() %>%

diff --git a/R/dm_from_con.R b/R/dm_from_con.R
@@ -64,11 +64,22 @@ dm_from_con <- function(
 
   src <- src_from_src_or_con(con)
 
+  # Use smart default for `.names`, if it wasn't provided
+  dots <- list2(...)
+  if (!is.null(.names)) {
+    names_pattern <- .names
+  } else if (is.null(dots$schema) || length(dots$schema) == 1) {
+    names_pattern <- "{.table}"
+  } else {
+    names_pattern <- "{.schema}.{.table}"
+    cli::cli_inform('Using {.code .names = "{names_pattern}"}')
+  }
+
   if (is.null(learn_keys) || isTRUE(learn_keys)) {
     # FIXME: Try to make it work everywhere
     tryCatch(
       {
-        dm_learned <- dm_learn_from_db(con, ...)
+        dm_learned <- dm_learn_from_db(con, ..., names_pattern = names_pattern)
         if (is_null(learn_keys)) {
           inform(c(
             "Keys queried successfully.",
@@ -104,7 +115,7 @@ dm_from_con <- function(
   }
 
   if (is_null(table_names)) {
-    src_tbl_names <- get_src_tbl_names(src, ..., names = .names)
+    src_tbl_names <- get_src_tbl_names(src, ..., names_pattern = names_pattern)
   } else {
     src_tbl_names <- table_names
     if (is.null(names(src_tbl_names))) {

diff --git a/R/learn.R b/R/learn.R
@@ -33,7 +33,7 @@
 #'   iris_dm_learned <- dm_learn_from_db(src_sqlite)
 #' }
 #' @autoglobal
-dm_learn_from_db <- function(dest, dbname = NA, schema = NULL, name_format = "{table}") {
+dm_learn_from_db <- function(dest, dbname = NA, schema = NULL, names_pattern = "{.table}") {
   # assuming that we will not try to learn from (globally) temporary tables, which do not appear in sys.table
   con <- con_from_src_or_con(dest)
   src <- src_from_src_or_con(dest)
@@ -51,8 +51,8 @@ dm_learn_from_db <- function(dest, dbname = NA, schema = NULL, name_format = "{t
 
   dm_name <-
     df_info$tables %>%
-    select(catalog = table_catalog, schema = table_schema, table = table_name) %>%
-    mutate(name = glue(!!name_format)) %>%
+    select(catalog = table_catalog, .schema = table_schema, .table = table_name) %>%
+    mutate(name = glue(!!names_pattern)) %>%
     pull() %>%
     unclass() %>%
     vec_as_names(repair = "unique")