Skip to content

Commit

Permalink
Give useful error if probably missing I() (#1459)
Browse files Browse the repository at this point in the history
Fixes #1451
  • Loading branch information
hadley authored Feb 21, 2024
1 parent 5b07997 commit 6b730e9
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
22 changes: 18 additions & 4 deletions R/tbl-sql.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,32 @@ tbl_sql <- function(subclass,
check_from = deprecated()) {
# Can't use check_dots_used(), #1429
check_character(vars, allow_null = TRUE)

if (lifecycle::is_present(check_from)) {
lifecycle::deprecate_warn("2.5.0", "tbl_sql(check_from)")
}

from <- as_table_source(from, con = src$con)
vars <- vars %||% dbplyr_query_fields(src$con, from)
is_suspicious <- is_bare_string(from) && grepl(".", from, fixed = TRUE)
source <- as_table_source(from, con = src$con)

withCallingHandlers(
vars <- vars %||% dbplyr_query_fields(src$con, source),
error = function(err) {
if (!is_suspicious) return()

cli::cli_abort(
c(
"Failed to find table {source}.",
i = "Did you mean {.code from = I({.str {from}})}?"
),
parent = err
)
}
)

dplyr::make_tbl(
c(subclass, "sql", "lazy"),
src = src,
lazy_query = lazy_query_remote(from, vars)
lazy_query = lazy_query_remote(source, vars)
)
}

Expand Down
14 changes: 14 additions & 0 deletions tests/testthat/_snaps/tbl-sql.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,20 @@
2 2 2
3 3 1

# useful error if missing I()

Code
tbl(src_memdb(), "foo.bar")
Condition
Error in `tbl_sql()`:
! Failed to find table `foo.bar`.
i Did you mean `from = I("foo.bar")`?
Caused by error in `db_query_fields.DBIConnection()`:
! Can't query fields.
i Using SQL: SELECT * FROM `foo.bar` AS `q05` WHERE (0 = 1)
Caused by error:
! no such table: foo.bar

# check_from is deprecated

Code
Expand Down
7 changes: 7 additions & 0 deletions tests/testthat/test-tbl-sql.R
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ test_that("can distinguish 'schema.table' from 'schema'.'table'", {
expect_equal(as.character(tbl_vars(df)), c("a", "b", "c"))
})

test_that("useful error if missing I()", {
expect_snapshot(
tbl(src_memdb(), "foo.bar"),
error = TRUE
)
})

test_that("check_from is deprecated", {
con <- local_sqlite_connection()
DBI::dbExecute(con, "CREATE TABLE x (y)")
Expand Down

0 comments on commit 6b730e9

Please sign in to comment.