Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dbWriteTable Inconsistently accepts DBI:ID #457

Open
kmishra9 opened this issue Mar 30, 2023 · 1 comment
Open

dbWriteTable Inconsistently accepts DBI:ID #457

kmishra9 opened this issue Mar 30, 2023 · 1 comment

Comments

@kmishra9
Copy link

While doing some bug hunting for RPostgres, realized there's also probably a bug in RSQLite when writing from a CSV:

# ******************************************************************************
# 1. Set Up ####
# ******************************************************************************

suppressPackageStartupMessages(library(dplyr))
suppressPackageStartupMessages(library(dbplyr))

db1 <- DBI::dbConnect(RSQLite::SQLite(), dbname = ":memory:")
db2 <- DBI::dbConnect(RSQLite::SQLite(), dbname = ":memory:")

test_df_full <- nycflights13::flights
test_df_lite <- nycflights13::flights %>% slice(1:100)
    
temp_path_full <- tempfile()
temp_path_lite <- tempfile()

db_path_full_sq <- DBI::Id(schema = NULL, table = 'test_df_full')
db_path_lite_sq <- DBI::Id(schema = NULL, table = 'test_df_lite')

readr::write_csv(x = test_df_full, file = temp_path_full)
readr::write_csv(x = test_df_lite, file = temp_path_lite)

# ******************************************************************************
# 2. Testing DBI:Id with Local DF ####
# ******************************************************************************

# Both work in SQLite and Microsoft SQL though
DBI::dbWriteTable(conn = db1, name = db_path_full_sq, value = test_df_full, overwrite = T)
DBI::dbWriteTable(conn = db1, name = db_path_lite_sq, value = test_df_lite, overwrite = T)

db1 %>% tbl(db_path_full_sq) %>% tally() # Expected: n = 336,776; Actual: 336,776
#> # Source:   SQL [1 x 1]
#> # Database: sqlite 3.40.1 [:memory:]
#>        n
#>    <int>
#> 1 336776
db1 %>% tbl(db_path_lite_sq) %>% tally() # Expected: n = 100; Actual: n = 100
#> # Source:   SQL [1 x 1]
#> # Database: sqlite 3.40.1 [:memory:]
#>       n
#>   <int>
#> 1   100

# ******************************************************************************
# 3. Testing DBI:Id with CSV ####
# ******************************************************************************

# Fails (Shouldn't)
DBI::dbWriteTable(conn = db2, name = db_path_full_sq, value = temp_path_full, overwrite = T)
#> Error in connection_import_file(conn@ptr, name, value, sep, eol, skip): RS_sqlite_import: no such table: `test_df_full`
DBI::dbWriteTable(conn = db2, name = db_path_lite_sq , value = temp_path_lite, overwrite = T)
#> Error in connection_import_file(conn@ptr, name, value, sep, eol, skip): RS_sqlite_import: no such table: `test_df_lite`

db2 %>% tbl(db_path_full_sq) %>% tally() # Expected: n = 336,776; Actual: Error
#> Error in `db_query_fields.DBIConnection()`:
#> ! Can't query fields.
#> Caused by error:
#> ! no such table: test_df_full
#> Backtrace:
#>      ▆
#>   1. ├─db2 %>% tbl(db_path_full_sq) %>% tally()
#>   2. ├─dplyr::tally(.)
#>   3. ├─dplyr::tbl(., db_path_full_sq)
#>   4. └─dplyr:::tbl.DBIConnection(., db_path_full_sq)
#>   5.   ├─dplyr::tbl(...)
#>   6.   └─dbplyr:::tbl.src_dbi(...)
#>   7.     └─dbplyr::tbl_sql(c(subclass, "dbi"), src = src, from = from, ...)
#>   8.       ├─vars %||% dbplyr_query_fields(src$con, from_sql)
#>   9.       └─dbplyr:::dbplyr_query_fields(src$con, from_sql)
#>  10.         └─dbplyr:::dbplyr_fallback(con, "db_query_fields", ...)
#>  11.           ├─rlang::eval_bare(expr((!!fun)(con, ...)))
#>  12.           └─dbplyr:::db_query_fields.DBIConnection(con, ...)
#>  13.             └─base::tryCatch(...)
#>  14.               └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>  15.                 └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>  16.                   └─value[[3L]](cond)
#>  17.                     └─cli::cli_abort("Can't query fields.", parent = cnd)
#>  18.                       └─rlang::abort(...)
db2 %>% tbl(db_path_lite_sq) %>% tally() # Expected: n = 100; Actual: Error
#> Error in `db_query_fields.DBIConnection()`:
#> ! Can't query fields.
#> Caused by error:
#> ! no such table: test_df_lite
#> Backtrace:
#>      ▆
#>   1. ├─db2 %>% tbl(db_path_lite_sq) %>% tally()
#>   2. ├─dplyr::tally(.)
#>   3. ├─dplyr::tbl(., db_path_lite_sq)
#>   4. └─dplyr:::tbl.DBIConnection(., db_path_lite_sq)
#>   5.   ├─dplyr::tbl(...)
#>   6.   └─dbplyr:::tbl.src_dbi(...)
#>   7.     └─dbplyr::tbl_sql(c(subclass, "dbi"), src = src, from = from, ...)
#>   8.       ├─vars %||% dbplyr_query_fields(src$con, from_sql)
#>   9.       └─dbplyr:::dbplyr_query_fields(src$con, from_sql)
#>  10.         └─dbplyr:::dbplyr_fallback(con, "db_query_fields", ...)
#>  11.           ├─rlang::eval_bare(expr((!!fun)(con, ...)))
#>  12.           └─dbplyr:::db_query_fields.DBIConnection(con, ...)
#>  13.             └─base::tryCatch(...)
#>  14.               └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>  15.                 └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>  16.                   └─value[[3L]](cond)
#>  17.                     └─cli::cli_abort("Can't query fields.", parent = cnd)
#>  18.                       └─rlang::abort(...)

# Works
DBI::dbWriteTable(conn = db2, name = 'test_df_full', value = temp_path_full, overwrite = T)
DBI::dbWriteTable(conn = db2, name = 'test_df_lite', value = temp_path_lite, overwrite = T)

db2 %>% tbl(db_path_full_sq) %>% tally() # Expected: n = 336,776; Actual: 336,776
#> # Source:   SQL [1 x 1]
#> # Database: sqlite 3.40.1 [:memory:]
#>        n
#>    <int>
#> 1 336776
db2 %>% tbl(db_path_lite_sq) %>% tally() # Expected: n = 100; Actual: n = 100
#> # Source:   SQL [1 x 1]
#> # Database: sqlite 3.40.1 [:memory:]
#>       n
#>   <int>
#> 1   100

Created on 2023-03-30 with reprex v2.0.2

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2023

Thanks, good catch.

It seems that the RS_sqlite_import() C function uses the %q format specifier to construct SQL with the table name quoted. We could change that to use %s in that function and call dbQuoteIdentifier() before entering RS_sqlite_import() .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants