feat: dm_from_con() can use multiple schemata
#1449
Conversation
| @@ -241,7 +245,7 @@ filter_dm_meta <- function(dm_meta, catalog = NULL, schema = NULL) { | |||
| constraint_column_usage <- constraint_column_usage %>% filter(table_catalog %in% !!catalog) | |||
| } | |||
|
|
|||
| if (!is.null(schema) && !is.na(schema)) { | |||
| if (!is.null(schema) && isTRUE(!is.na(schema))) { | |||
| schemata <- schemata %>% filter(schema_name %in% !!schema) | |||
| tables <- tables %>% filter(table_schema %in% !!schema) | |||
| columns <- columns %>% filter(table_schema %in% !!schema) | |||
There was a problem hiding this comment.
The condition below
} else if (!isTRUE(is.na(schema)) && is_mariadb(dm_get_con(dm_meta))) {
is in my opinion confusing. I think the idea is to use a default for MariaDB if schema is NULL. If so, this should use is.null(schema). Also, it might be better to implement these defaults differently. There is a function schema_mariadb() which seems to implement this default. But it always uses database() and here the default handling is different in case database() is NULL.
There was a problem hiding this comment.
IIRC DATABASE() can be NULL in MariaDB/MySQL if connecting without a default database, and this could be the reason for this ... hack?
| expect_snapshot_output(my_dm <- dm_from_con(my_db, schema = c("Accidents", "Ad"))) | ||
| expect_snapshot(dm::dm_get_all_fks(my_dm)) | ||
| expect_snapshot(dm::dm_get_all_pks(my_dm)) |
There was a problem hiding this comment.
The test output looks odd, perhaps because the connection uses dbname = "Financial_ijs" ?
I wonder what's a good test design on Postgres and MSSQL for this feature.
There was a problem hiding this comment.
Maybe simply setup our own DB.
Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>
krlmlr
changed the title
dm_from_con()dm_from_con() can use multiple schemata
|
Thanks! |
Fixes #1441.