diff --git a/DESCRIPTION b/DESCRIPTION index 2b7ac3f..d6844b3 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -40,4 +40,4 @@ SystemRequirements: Impala driver to support a 'DBI'-compatible R interface NeedsCompilation: no License: Apache License 2.0 | file LICENSE Encoding: UTF-8 -RoxygenNote: 6.1.1 +RoxygenNote: 7.1.1 diff --git a/NAMESPACE b/NAMESPACE index db374e1..028a32e 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -36,6 +36,7 @@ importFrom(DBI,dbExecute) importFrom(DBI,dbFetch) importFrom(DBI,dbGetInfo) importFrom(DBI,dbGetQuery) +importFrom(DBI,dbQuoteIdentifier) importFrom(DBI,dbSendQuery) importFrom(assertthat,assert_that) importFrom(assertthat,is.flag) diff --git a/NEWS.md b/NEWS.md index 0412124..b7e533f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,9 @@ # implyr (development version) +* Fixed bugs in table creation (#47, @karoliskascenas) * Allow user to set `copy_to()` size limit with option `implyr.copy_to_size_limit` (#49, @karoliskascenas) * Added more SQL translations +* Fixed a DBI identifier quoting problem causing errors with dbplyr 2.0.0 (#48) # implyr 0.3.0 diff --git a/R/db-impala.R b/R/db-impala.R index 36b9d2f..0390464 100644 --- a/R/db-impala.R +++ b/R/db-impala.R @@ -94,6 +94,16 @@ sql_escape_ident.impala_connection <- function(con, x) { impala_escape_ident(con, x, "`") } +#' @importFrom DBI dbQuoteIdentifier +#' @importFrom methods setMethod +setMethod("dbQuoteIdentifier", c("impala_connection", "character"), function(conn, x, ...) { + impala_escape_ident(conn, x, "`") +}) + +setMethod("dbQuoteIdentifier", c("impala_connection", "ident"), function(conn, x, ...) { + impala_escape_ident(conn, x, "`") +}) + #' @export #' @importFrom dplyr sql_escape_string sql_escape_string.impala_connection <- function(con, x) { @@ -597,7 +607,7 @@ db_commit.impala_connection <- function(con, ...) { #' @export #' @importFrom dplyr db_analyze db_analyze.impala_connection <- function(con, table, ...) { - sql <- build_sql("COMPUTE STATS", ident(table), con = con) + sql <- build_sql("COMPUTE STATS ", ident(table), con = con) dbExecute(con, sql) } @@ -709,6 +719,12 @@ db_create_table.impala_connection <- is_nchar_one_string_or_null(field_terminator), is_nchar_one_string_or_null(line_terminator) ) + if (isTRUE(grepl("^[[:cntrl:]]{1}$", field_terminator))) { + field_terminator <- gsub("\"", "", deparse(field_terminator), fixed = TRUE) + } + if (isTRUE(grepl("^[[:cntrl:]]{1}$", line_terminator))) { + line_terminator <- gsub("\"", "", deparse(line_terminator), fixed = TRUE) + } if (temporary) { stop( "Impala does not support temporary tables. Set temporary = FALSE in db_create_table().", @@ -733,6 +749,8 @@ db_create_table.impala_connection <- }, ident(table), " ", + fields, + " ", if (!is.null(field_terminator) || !is.null(line_terminator)) { sql("ROW FORMAT DELIMITED ") @@ -746,7 +764,6 @@ db_create_table.impala_connection <- if (!is.null(file_format)) { sql(paste0("STORED AS ", file_format, " ")) }, - fields, con = con) dbExecute(con, sql) } diff --git a/R/src_impala.R b/R/src_impala.R index cad752f..7ac5a87 100644 --- a/R/src_impala.R +++ b/R/src_impala.R @@ -332,7 +332,7 @@ copy_to.src_impala <- temporary = TRUE, unique_indexes = NULL, indexes = NULL, - analyze = TRUE, + analyze = FALSE, external = FALSE, force = FALSE, field_terminator = NULL, @@ -385,7 +385,7 @@ copy_to.src_impala <- external = external, force = force, field_terminator = field_terminator, - line_terminator = field_terminator, + line_terminator = line_terminator, file_format = file_format, ... ) diff --git a/tests/testthat/test-cast.R b/tests/testthat/test-cast.R index 33715b3..8d0a54a 100644 --- a/tests/testthat/test-cast.R +++ b/tests/testthat/test-cast.R @@ -25,6 +25,7 @@ test_that("as.string() returns column of type string", { }) test_that("as.char() returns column of type char with specified length", { + skip("Skipping because some versions of JDBC driver transform to string") check_impala() expect_equal( column_type(tbl(impala, "flights") %>% transmute(year = as.char(year, 4))), @@ -122,6 +123,7 @@ test_that("as.double() returns column of type double", { # do not test as.real(); might return float or double test_that("as.float() returns column of type float", { + skip("Skipping because some versions of JDBC driver transform to double") check_impala() expect_equal( column_type( @@ -132,6 +134,7 @@ test_that("as.float() returns column of type float", { }) test_that("as.single() returns column of type float", { + skip("Skipping because some versions of JDBC driver transform to double") check_impala() expect_equal( column_type( diff --git a/tests/testthat/test-copy_to.R b/tests/testthat/test-copy_to.R index f9f9b72..5c2b43d 100644 --- a/tests/testthat/test-copy_to.R +++ b/tests/testthat/test-copy_to.R @@ -7,7 +7,14 @@ test_that("copy_to() succeeds on a very small data frame", { table_name <- paste0(sample(letters, 10, replace = TRUE), collapse = "") dbExecute(impala, paste0("DROP TABLE IF EXISTS ", table_name)) Sys.sleep(2) - copy_to(impala, nycflights13::airlines[1:5, ], name = table_name, temporary = FALSE) + copy_to( + impala, + nycflights13::airlines[1:5, ], + name = table_name, + temporary = FALSE, + field_terminator = "\t", + line_terminator = "\n" + ) airlines_impala <- dbGetQuery(impala, paste0("SELECT * FROM ", table_name, " ORDER BY carrier")) # right-trim variable length strings because RJDBC pads them with spaces