Skip to content

Commit

Permalink
Merge pull request #1127 from cynkra/f-learn-mysql-2-tweaks
Browse files Browse the repository at this point in the history
 use helper to centralize info about schema-supported databases
  • Loading branch information
krlmlr authored Jul 5, 2022
2 parents 6f17152 + b06cca5 commit 0137880
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 10 deletions.
17 changes: 16 additions & 1 deletion R/db-helpers.R
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,21 @@ is_mariadb <- function(dest) {
inherits_any(dest, c("MariaDBConnection", "src_MariaDBConnection", "src_DoltConnection", "src_DoltLocalConnection"))
}

schema_supported_dbs <- function() {
tibble::tribble(
~db_name, ~id_function, ~test_shortcut,
"SQL Server", "is_mssql", "mssql",
"Postgres", "is_postgres", "postgres",
"MariaDB", "is_mariadb", "maria",
)
}

is_schema_supported <- function(con) {
funs <- schema_supported_dbs()[["id_function"]]

any(purrr::map_lgl(funs, ~ do.call(., args = list(dest = con))))
}

src_from_src_or_con <- function(dest) {
if (is.src(dest)) dest else dbplyr::src_dbi(dest)
}
Expand All @@ -90,7 +105,7 @@ repair_table_names_for_db <- function(table_names, temporary, con, schema = NULL
names <- unique_db_table_name(names)
} else {
# permanent tables
if (!is.null(schema) && !is_mssql(con) && !is_postgres(con) && !is_mariadb(con)) {
if (!is.null(schema) && !is_schema_supported(con)) {
abort_no_schemas_supported(con = con)
}
names <- table_names
Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/helper-skip.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ skip_if_src_not <- function(...) {
}
}

skip_if_schema_supported <- function() {
not_ok_dbs <- schema_supported_dbs()[["test_shortcut"]]
skip_if_src(not_ok_dbs)
}
skip_if_schema_not_supported <- function() {
not_ok_dbs <- schema_supported_dbs()[["test_shortcut"]]
skip_if_src_not(not_ok_dbs)
}

skip_if_ide <- function() {
if (!isTRUE(as.logical(Sys.getenv("CI")))) {
skip("Slow test. To run, set CI=true")
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-db-interface.R
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ test_that("copy_dm_to() works with schema argument for MSSQL & Postgres", {
)
})

test_that("copy_dm_to() fails with schema argument for databases other than MSSQL & Postgres", {
skip_if_src("mssql", "postgres", "maria")
test_that("copy_dm_to() fails with schema argument for databases where schema is unsupported", {
skip_if_schema_supported()

local_dm <- dm_for_filter() %>% collect()

Expand Down
8 changes: 4 additions & 4 deletions tests/testthat/test-learn.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# FIXME: #313: learn only from current source

test_that("Standard learning from MSSQL (schema 'dbo') or Postgres (schema 'public') and get_src_tbl_names() works?", {
skip_if_src_not(c("mssql", "postgres", "maria"))
skip_if_schema_not_supported()

# dm_learn_from_mssql() --------------------------------------------------
con_db <- my_test_con()
Expand Down Expand Up @@ -42,7 +42,7 @@ test_that("Standard learning from MSSQL (schema 'dbo') or Postgres (schema 'publ
})

test_that("Standard learning from MSSQL (schema 'dbo') or Postgres (schema 'public') and get_src_tbl_names() works?", {
skip_if_src_not(c("mssql", "postgres", "maria"))
skip_if_schema_not_supported()

# dm_learn_from_mssql() --------------------------------------------------
con_db <- my_test_con()
Expand Down Expand Up @@ -109,7 +109,7 @@ test_that("Standard learning from MSSQL (schema 'dbo') or Postgres (schema 'publ


test_that("Learning from specific schema on MSSQL or Postgres works?", {
skip_if_src_not(c("mssql", "postgres", "maria"))
skip_if_schema_not_supported()

# produces a randomized schema name with a length of 4-10 characters
# consisting of the symbols in `reservoir`
Expand Down Expand Up @@ -406,7 +406,7 @@ test_that("Learning from a specific schema in another DB for MSSQL works?", {
# Must be in the same file as the other learning tests
# to avoid race conditions.
test_that("dm_meta() contents", {
skip_if_src_not(c("mssql", "postgres", "maria"))
skip_if_schema_not_supported()

# produces a randomized schema name with a length of 4-10 characters
# consisting of the symbols in `reservoir`
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-meta.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ test_that("dummy", {
})

test_that("dm_meta() data model", {
skip_if_src_not(c("mssql", "postgres", "maria"))
skip_if_schema_not_supported()

expect_snapshot({
dm_meta(my_test_src()) %>%
Expand Down
4 changes: 2 additions & 2 deletions vignettes/dm.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ fin_dm
``````

The dm object interrogates the RDBMS for table and column information, and primary and foreign keys.
Currently, primary and foreign keys are only available from `r glue::glue_collapse(dm:::schema_supported_dbs()[["db_name"]], sep = ", ", last = " and ")`.


## Selecting tables
Expand All @@ -77,7 +78,6 @@ fin_dm_small <-
dm_select_tbl(loans, accounts, districts, trans)
```


## Linking tables by adding keys

In many cases, `dm_from_con()` already returns a dm with all keys set.
Expand All @@ -92,7 +92,7 @@ fin_dm_small <-
dm_select_tbl(loans, accounts, districts, trans)
``````

In our data model, `id` columns uniquely identify records in the `accounts` and `loans` tables, and can be used as a primary key.
In our data model, `id` columns uniquely identify records in the `accounts` and `loans` tables, and was used as a primary key.
A primary key is defined with `dm_add_pk()`.
Each loan is linked to one account via the `account_id` column in the `loans` table, the relationship is established with `dm_add_fk()`.

Expand Down

0 comments on commit 0137880

Please sign in to comment.