From 1037df3b899097f93d0575f922966673bc002377 Mon Sep 17 00:00:00 2001
From: owenjonesuob <21007837+owenjonesuob@users.noreply.github.com>
Date: Fri, 3 May 2024 19:26:15 +0100
Subject: [PATCH] fix: pass naming pattern through to dm_learn_from_db()
 (#2213)

---
 R/db-helpers.R  | 11 +----------
 R/dm_from_con.R | 15 +++++++++++++--
 R/learn.R       |  6 +++---
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/R/db-helpers.R b/R/db-helpers.R
index 4ec793a53..6413b424e 100644
--- a/R/db-helpers.R
+++ b/R/db-helpers.R
@@ -124,6 +124,7 @@ find_name_clashes <- function(old, new) {
 get_src_tbl_names <- function(src, schema = NULL, dbname = NULL, names = NULL) {
   if (!is_mssql(src) && !is_postgres(src) && !is_mariadb(src)) {
     warn_if_arg_not(schema, only_on = c("MSSQL", "Postgres", "MariaDB"))
+get_src_tbl_names <- function(src, schema = NULL, dbname = NULL, names_pattern = "{.table}") {
     warn_if_arg_not(dbname, only_on = "MSSQL")
     tables <- src_tbls(src)
     out <- purrr::map(tables, ~ DBI::Id(table = .x))
@@ -155,16 +156,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
index 21a20041a..5b0eb1482 100644
--- 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
index 4deb1cf65..19d00ff9d 100644
--- 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")