From c21ac194fea8da45c88229c6a7a1fb8864b6b861 Mon Sep 17 00:00:00 2001 From: Josh Schoenbachler Date: Mon, 18 Jan 2021 13:36:19 -0600 Subject: [PATCH 1/7] replace dbAppendTable code with code from dbWriteTable when append = TRUE and copy = TRUE --- R/tables.R | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/R/tables.R b/R/tables.R index 0897817e..26b3c6c1 100644 --- a/R/tables.R +++ b/R/tables.R @@ -191,17 +191,19 @@ setMethod("dbAppendTable", c("PqConnection"), function(conn, name, value, ..., row.names = NULL) { stopifnot(is.null(row.names)) - query <- sqlAppendTableTemplate( - con = conn, - table = name, - values = value, - row.names = row.names, - prefix = "$", - pattern = "1", - ... - ) + row.names <- FALSE + + value <- sqlRownamesToColumn(value, row.names) - dbExecute(conn, query, params = unname(as.list(value))) + value <- sql_data_copy(value, row.names = FALSE) + + fields <- dbQuoteIdentifier(conn, names(value)) + sql <- paste0( + "COPY ", dbQuoteIdentifier(conn, name), + " (", paste(fields, collapse = ", "), ")", + " FROM STDIN" + ) + connection_copy_data(conn@ptr, sql, value) } ) From b8c68c86150fbd36031cec60c861eca79023cdfb Mon Sep 17 00:00:00 2001 From: Josh Schoenbachler Date: Fri, 22 Jan 2021 10:03:16 -0600 Subject: [PATCH 2/7] Return amount of rows affected --- R/tables.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/tables.R b/R/tables.R index 26b3c6c1..1aeea65f 100644 --- a/R/tables.R +++ b/R/tables.R @@ -204,6 +204,8 @@ setMethod("dbAppendTable", c("PqConnection"), " FROM STDIN" ) connection_copy_data(conn@ptr, sql, value) + + nrow(value) } ) From 94ac2b74ecb2f97ebd33f97b9b2234fd1e3e9f1a Mon Sep 17 00:00:00 2001 From: Josh Schoenbachler Date: Fri, 22 Jan 2021 11:01:56 -0600 Subject: [PATCH 3/7] Add warning if factor. --- R/tables.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/R/tables.R b/R/tables.R index 1aeea65f..19e39d1a 100644 --- a/R/tables.R +++ b/R/tables.R @@ -193,6 +193,11 @@ setMethod("dbAppendTable", c("PqConnection"), row.names <- FALSE + is_factor = vlapply(value, is.factor) + if (any(is_factor)) { + warning("Factors converted to character", call. = FALSE) + } + value <- sqlRownamesToColumn(value, row.names) value <- sql_data_copy(value, row.names = FALSE) From 0d8e2d191b2b393eb054f075fa488f4e291a02bc Mon Sep 17 00:00:00 2001 From: Josh Schoenbachler Date: Fri, 22 Jan 2021 12:08:24 -0600 Subject: [PATCH 4/7] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kirill Müller --- R/tables.R | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/R/tables.R b/R/tables.R index 19e39d1a..a8f0ab7a 100644 --- a/R/tables.R +++ b/R/tables.R @@ -193,10 +193,7 @@ setMethod("dbAppendTable", c("PqConnection"), row.names <- FALSE - is_factor = vlapply(value, is.factor) - if (any(is_factor)) { - warning("Factors converted to character", call. = FALSE) - } + value = factor_to_string(value) value <- sqlRownamesToColumn(value, row.names) From 07e08139f649028ad9f22881d5bae9de10952d5c Mon Sep 17 00:00:00 2001 From: Josh Schoenbachler Date: Fri, 22 Jan 2021 14:55:21 -0600 Subject: [PATCH 5/7] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kirill Müller --- R/tables.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/tables.R b/R/tables.R index a8f0ab7a..b4da2608 100644 --- a/R/tables.R +++ b/R/tables.R @@ -193,7 +193,7 @@ setMethod("dbAppendTable", c("PqConnection"), row.names <- FALSE - value = factor_to_string(value) + value = factor_to_string(value, warn = TRUE) value <- sqlRownamesToColumn(value, row.names) From 52bf97791656bff68c7aa6eece683dccfee98f61 Mon Sep 17 00:00:00 2001 From: Josh Schoenbachler Date: Mon, 25 Jan 2021 16:43:08 -0600 Subject: [PATCH 6/7] Update with feedback from pull request --- R/tables.R | 44 ++++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/R/tables.R b/R/tables.R index b4da2608..721bec0b 100644 --- a/R/tables.R +++ b/R/tables.R @@ -108,22 +108,7 @@ setMethod("dbWriteTable", c("PqConnection", "character", "data.frame"), } if (nrow(value) > 0) { - if (!copy) { - value <- sqlData(conn, value, row.names = FALSE) - - sql <- sqlAppendTable(conn, name, value, row.names = FALSE) - dbExecute(conn, sql) - } else { - value <- sql_data_copy(value, row.names = FALSE) - - fields <- dbQuoteIdentifier(conn, names(value)) - sql <- paste0( - "COPY ", dbQuoteIdentifier(conn, name), - " (", paste(fields, collapse = ", "), ")", - " FROM STDIN" - ) - connection_copy_data(conn@ptr, sql, value) - } + dbAppendTable(conn, name, value, copy, warn = FALSE) } invisible(TRUE) @@ -188,24 +173,31 @@ format_keep_na <- function(x, ...) { #' @rdname postgres-tables #' @export setMethod("dbAppendTable", c("PqConnection"), - function(conn, name, value, ..., row.names = NULL) { + function(conn, name, value, copy = TRUE, warn = TRUE, ..., row.names = NULL) { stopifnot(is.null(row.names)) row.names <- FALSE - value = factor_to_string(value, warn = TRUE) + value = factor_to_string(value, warn = warn) value <- sqlRownamesToColumn(value, row.names) - value <- sql_data_copy(value, row.names = FALSE) + if (!copy) { + value <- sqlData(conn, value, row.names = FALSE) - fields <- dbQuoteIdentifier(conn, names(value)) - sql <- paste0( - "COPY ", dbQuoteIdentifier(conn, name), - " (", paste(fields, collapse = ", "), ")", - " FROM STDIN" - ) - connection_copy_data(conn@ptr, sql, value) + sql <- sqlAppendTable(conn, name, value, row.names = FALSE) + dbExecute(conn, sql) + } else { + value <- sql_data_copy(value, row.names = FALSE) + + fields <- dbQuoteIdentifier(conn, names(value)) + sql <- paste0( + "COPY ", dbQuoteIdentifier(conn, name), + " (", paste(fields, collapse = ", "), ")", + " FROM STDIN" + ) + connection_copy_data(conn@ptr, sql, value) + } nrow(value) } From f3490ac04dac3475b8915200e95f11b6871373f7 Mon Sep 17 00:00:00 2001 From: Josh Schoenbachler Date: Tue, 26 Jan 2021 10:49:37 -0600 Subject: [PATCH 7/7] Add param for 'warn' and run 'devtools::document()' --- R/tables.R | 1 + man/postgres-tables.Rd | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/R/tables.R b/R/tables.R index 721bec0b..19172634 100644 --- a/R/tables.R +++ b/R/tables.R @@ -24,6 +24,7 @@ #' and uses `COPY name FROM stdin`. This is fast, but not supported by #' all postgres servers (e.g. Amazon's redshift). If `FALSE`, generates #' a single SQL string. This is slower, but always supported. +#' @param warn If `TRUE`, warns user when converting from factor to string. #' #' @examples #' # For running the examples on systems without PostgreSQL connection: diff --git a/man/postgres-tables.Rd b/man/postgres-tables.Rd index 4bea47d5..24b821cf 100644 --- a/man/postgres-tables.Rd +++ b/man/postgres-tables.Rd @@ -30,7 +30,15 @@ \S4method{sqlData}{PqConnection}(con, value, row.names = FALSE, ...) -\S4method{dbAppendTable}{PqConnection}(conn, name, value, ..., row.names = NULL) +\S4method{dbAppendTable}{PqConnection}( + conn, + name, + value, + copy = TRUE, + warn = TRUE, + ..., + row.names = NULL +) \S4method{dbReadTable}{PqConnection,character}(conn, name, ..., check.names = TRUE, row.names = FALSE) @@ -90,6 +98,8 @@ a single SQL string. This is slower, but always supported.} \item{con}{A database connection.} +\item{warn}{If \code{TRUE}, warns user when converting from factor to string.} + \item{check.names}{If \code{TRUE}, the default, column names will be converted to valid R identifiers.}