Merge branch 'master' into patch-2

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

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)

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
 

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)
 }

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,
         ...
       )

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(

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