Use default schema in dm_learn_from_db()
#1448
Conversation
| ident_q <- function(...) { | ||
| # to avoid dependency on dbplyr define `ident_q()` here (not exported from dplyr) | ||
| structure(c(...), class = c("ident_q", "ident", "character")) | ||
| } | ||
|
|
There was a problem hiding this comment.
We're using dbplyr::ident_q() elsewhere.
| schema_mariadb <- function(con, schema) { | ||
| if (is_null(schema)) { | ||
| schema <- sql("database()") | ||
| schema <- ident_q("database()") |
There was a problem hiding this comment.
What's the advantage of using ident_q() here?
There was a problem hiding this comment.
It works better with %in% !!...:
library(dplyr, warn.conflicts = FALSE)
library(dbplyr, warn.conflicts = FALSE)
lf <- lazy_frame(x = 1)
lf %>% filter(x %in% !!sql("1"))
#> <SQL>
#> SELECT *
#> FROM `df`
#> WHERE (`x` IN 1)
lf %>% filter(x %in% !!ident_q("1"))
#> <SQL>
#> SELECT *
#> FROM `df`
#> WHERE (`x` IN (1))Created on 2022-09-05 with reprex v2.0.2
| schema_postgres(con, schema) | ||
| } else if (is_mariadb(src)) { | ||
| # MariaDB | ||
| schema_mariadb(con, schema) |
There was a problem hiding this comment.
Weird, we're using a filter with DATABASE() elsewhere. Do we still need that?
|
@mgirlich: Would you like to do another iteration? |
|
Thank you for your patience. I think we want |
|
Scratch that, |
Fixes #1440.
There already is a test
test_that("Standard learning from MSSQL (schema 'dbo') or Postgres (schema 'public') and get_src_tbl_names() works?"but it seems it doesn't work correct. I had a quick look at it but wasn't quite sure how to fix it. @krlmlr maybe you want to have a look at the test.