From abda61d78fe387abf0170bfaef0dd670a9b31e03 Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:16:51 +0100 Subject: [PATCH 01/11] reset roxygen ver --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 0324e54..761e6af 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -17,7 +17,7 @@ Imports: glue Language: en-GB Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.2 +RoxygenNote: 7.3.1 Suggests: testthat (>= 2.1.0), mockery From ebad829cb82576efbdc437d22cbd68ab1cd3bbd8 Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:17:18 +0100 Subject: [PATCH 02/11] help and warnings --- R/create_replace_table.R | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/R/create_replace_table.R b/R/create_replace_table.R index 1cbd731..fa639ea 100644 --- a/R/create_replace_table.R +++ b/R/create_replace_table.R @@ -299,7 +299,7 @@ populate_staging_table <- function(db_params, "Failed to write staging", "data to database.\n", cond, .sep = " " - )) + ), call. = FALSE) } ) message(glue::glue( @@ -471,8 +471,11 @@ clean_row_names <- function(dataframe) { } -#' Write an R dataframe to SQL Server table optionally with system -#' versioning on. +#' Write an R dataframe to a SQL Server table +#' +#' Can create table optionally with system versioning on. However, extra +#' permissions may be required to drop or overwrite +#' system versioned tables so use this option with caution. #' #' @param server Server instance where SQL Server database running. #' @param database Name of SQL Server database where table will be written. @@ -487,9 +490,11 @@ clean_row_names <- function(dataframe) { #' @param batch_size Source R dataframe rows will be loaded into a staging SQL #' Server table in batches of this many rows at a time. #' @param versioned_table Create table with SQL Server system versioning. -#' Defaults to FALSE. If table already exists in DB will not -#' change existing versioning status. If overwriting an existing table -#' may receive permissions error to contact system admin. +#' Defaults to FALSE. If appending to existing table will not +#' change existing versioning status. If overwriting an existing +#' versioned table may receive permissions error due to dropping existing +#' versioned table failing. Contact a sysadmin to drop the +#' versioned table for you. #' #' @importFrom utils tail #' @export @@ -548,8 +553,8 @@ write_dataframe_to_db <- function(server, # If not appending and exists then inform that will be overwritten } else { warning(glue::glue( - "Existing database table: {schema}.{table_name}", - "was over-written.", + "Database table: {schema}.{table_name} already exists", + "attempting to drop and replace it...", .sep = " " ), call. = FALSE) # Drop the existing table From 1ef789bf2355e584e85e00419069786221c8e439 Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:17:48 +0100 Subject: [PATCH 03/11] stop call supress --- R/db_table_metadata.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/db_table_metadata.R b/R/db_table_metadata.R index d8d26b7..3cacb7f 100644 --- a/R/db_table_metadata.R +++ b/R/db_table_metadata.R @@ -28,7 +28,7 @@ db_table_metadata <- function(server, database, schema, table_name) { )) { stop(glue::glue( "Table: {schema}.{table_name} does not exist in the database." - )) + ), call. = FALSE) } sql <- paste0(" SET NOCOUNT ON; From bf86c65d2646e0629b04782d06191e64907c4a9d Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:18:07 +0100 Subject: [PATCH 04/11] warning revision --- R/drop_table.R | 49 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/R/drop_table.R b/R/drop_table.R index 42aa575..ce802e6 100644 --- a/R/drop_table.R +++ b/R/drop_table.R @@ -45,28 +45,26 @@ create_drop_sql <- function(server, output = TRUE ) if (is_versioned(check_df)) { - warning(glue::glue( - "Attempting to drop system versioned table.", - "If receive permissions error will need to", - "ask admin to drop this table for you mentioning", - "it is a temporal/system versioned table." - )) drop_sql <- create_drop_sql_versioned(schema, table_name) } else { # if not actually versioned: drop_sql <- create_drop_sql_nonversioned(schema, table_name) } - return(drop_sql) + return(list(drop_sql = drop_sql, versioned = is_versioned(check_df))) } -#' Drop SQL Server table from database. Check if versioned table and disable -#' versioning and drop history table too if so. +#' Drop SQL Server table from database +#' +#' Drop specified table. Check if versioned table. If so attempt to disable +#' versioning and drop history table too if so. Extra permissions may be +#' required to drop a versioned table so contact system admin if +#' receive an error showing this is the case. #' #' @param server Server and instance where SQL Server database found. #' @param database Database containing the table to be dropped. #' @param schema Name of schema containing table to be dropped. #' @param table_name Name of the table to be dropped. -#' @param versioned_table Is this a versioned table. Legacy parameter no -#' longer used as this is now checked every time. +#' @param versioned_table Is this a versioned table. Legacy argument no +#' longer used. This is now checked every time regardless of T or F input. #' @param silent If TRUE do not give message that dropping complete. #' Defaults to FALSE. #' @@ -108,12 +106,29 @@ drop_table_from_db <- function(server, ) # Run drop sql - - execute_sql( - server, - database, - drop_sql, - output = FALSE + tryCatch( + { + execute_sql( + server, + database, + drop_sql$drop_sql, + output = FALSE + ) + }, + error = function(cond) { + if (drop_sql$versioned) { + cond$message <- glue::glue( + "{cond$message}\n\n", + "{schema}.{table_name} is a VERSIONED TABLE.\n\n", + "Contact a system admin to request that they drop this versioned ", + "table for you as you do not have sufficient permissions.", + ) + } else { + cond$message <- glue::glue("Error dropping table: {cond}") + } + cond$call <- NULL + stop(cond) + } ) # Output message if required From cd2f0b7bd91ed0555153b8ecaef9a571b81c6430 Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:18:37 +0100 Subject: [PATCH 05/11] refactor execute --- R/execute_sql.R | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/R/execute_sql.R b/R/execute_sql.R index d354672..a8d79d6 100644 --- a/R/execute_sql.R +++ b/R/execute_sql.R @@ -23,30 +23,25 @@ execute_sql <- function(server, database, sql, output = FALSE) { server = server, database = database ) - if (output) { - tryCatch( - { + + tryCatch( + { + if (output) { output_data <- DBI::dbGetQuery(connection, sql) - }, - error = function(cond) { - stop(glue::glue( - "Failed to execute SQL.\nODBC error message: {cond}" - )) - } - ) - } else { - tryCatch( - { + } else { DBI::dbExecute(connection, sql) output_data <- glue::glue("SQL: {sql}\nexecuted successfully") - }, - error = function(cond) { - stop(glue::glue( - "Failed to execute SQL.\nODBC error message: {cond}" - )) } - ) - } - DBI::dbDisconnect(connection) + }, + error = function(cond) { + stop(glue::glue( + "Failed to execute SQL. ODBC error message:\n{cond$message}" + ), call. = FALSE) + }, + finally = { + DBI::dbDisconnect(connection) + } + ) + return(output_data) } From db27426780ef095a515187d5b28a85c1b3f69f15 Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:18:58 +0100 Subject: [PATCH 06/11] stop remove calls --- R/read_table.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/read_table.R b/R/read_table.R index e4dedd0..17d29ec 100644 --- a/R/read_table.R +++ b/R/read_table.R @@ -23,14 +23,14 @@ table_select_list <- function(server, if (length(not_exist_cols) > 0) { stop(glue::glue( "Column {not_exist_cols} not found in {schema}.{table_name}." - )) + ), call. = FALSE) } else { return(columns) } # where no user specified list of columns } else { - if (!include_pk & !is.null(pk)) { + if (!include_pk && !is.null(pk)) { return(existing_cols[existing_cols != pk]) } else { return(existing_cols) @@ -163,7 +163,7 @@ read_table_from_db <- function(database, )) { stop(glue::glue( "Table: {schema}.{table_name} does not exist in the database." - )) + ), call. = FALSE) } # Use a genuine connection, so the filter translation SQL is correct From ed2d6e3116b01ec31987d6146552dc4b78bb96c2 Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:19:13 +0100 Subject: [PATCH 07/11] stop remove calls --- R/utils.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index 6af40f2..fd5c5ed 100644 --- a/R/utils.R +++ b/R/utils.R @@ -16,7 +16,7 @@ create_sqlserver_connection <- function(server, database, timeout = 10) { "{database} on server: {server}", "\n{cond}", .sep = " " - )) + ), call. = FALSE) } ) } From 38b6e4f924c3a66af7bcc39b864a7ef525a7ef32 Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:19:56 +0100 Subject: [PATCH 08/11] update documentation --- man/drop_table_from_db.Rd | 13 +++++++------ man/write_dataframe_to_db.Rd | 16 +++++++++------- tests/testthat/test-drop_table.R | 6 +++--- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/man/drop_table_from_db.Rd b/man/drop_table_from_db.Rd index 257ded0..931ac99 100644 --- a/man/drop_table_from_db.Rd +++ b/man/drop_table_from_db.Rd @@ -2,8 +2,7 @@ % Please edit documentation in R/drop_table.R \name{drop_table_from_db} \alias{drop_table_from_db} -\title{Drop SQL Server table from database. Check if versioned table and disable -versioning and drop history table too if so.} +\title{Drop SQL Server table from database} \usage{ drop_table_from_db( server, @@ -23,15 +22,17 @@ drop_table_from_db( \item{table_name}{Name of the table to be dropped.} -\item{versioned_table}{Is this a versioned table. Legacy parameter no -longer used as this is now checked every time.} +\item{versioned_table}{Is this a versioned table. Legacy argument no +longer used. This is now checked every time regardless of T or F input.} \item{silent}{If TRUE do not give message that dropping complete. Defaults to FALSE.} } \description{ -Drop SQL Server table from database. Check if versioned table and disable -versioning and drop history table too if so. +Drop specified table. Check if versioned table. If so attempt to disable +versioning and drop history table too if so. Extra permissions may be +required to drop a versioned table so contact system admin if +receive an error showing this is the case. } \examples{ \dontrun{ diff --git a/man/write_dataframe_to_db.Rd b/man/write_dataframe_to_db.Rd index 66a9b33..b511c9e 100644 --- a/man/write_dataframe_to_db.Rd +++ b/man/write_dataframe_to_db.Rd @@ -2,8 +2,7 @@ % Please edit documentation in R/create_replace_table.R \name{write_dataframe_to_db} \alias{write_dataframe_to_db} -\title{Write an R dataframe to SQL Server table optionally with system -versioning on.} +\title{Write an R dataframe to a SQL Server table} \usage{ write_dataframe_to_db( server, @@ -37,13 +36,16 @@ database table (if exists). Default FALSE.} Server table in batches of this many rows at a time.} \item{versioned_table}{Create table with SQL Server system versioning. -Defaults to FALSE. If table already exists in DB will not -change existing versioning status. If overwriting an existing table -may receive permissions error to contact system admin.} +Defaults to FALSE. If appending to existing table will not +change existing versioning status. If overwriting an existing +versioned table may receive permissions error due to dropping existing +versioned table failing. Contact a sysadmin to drop the +versioned table for you.} } \description{ -Write an R dataframe to SQL Server table optionally with system -versioning on. +Can create table optionally with system versioning on. However, extra +permissions may be required to drop or overwrite +system versioned tables so use this option with caution. } \examples{ \dontrun{ diff --git a/tests/testthat/test-drop_table.R b/tests/testthat/test-drop_table.R index bfc7a09..035cacb 100644 --- a/tests/testthat/test-drop_table.R +++ b/tests/testthat/test-drop_table.R @@ -20,7 +20,7 @@ test_that("versioned drop sql created correctly", { expect_equal(create_drop_sql( "test", "test", "test_schema", "test_tbl" - ), drop_ver_sql) + )$drop_sql, drop_ver_sql) }) test_that("non-versioned drop sql created correctly", { @@ -34,7 +34,7 @@ test_that("non-versioned drop sql created correctly", { expect_equal(create_drop_sql( "test", "test", "test_schema", "test_tbl" - ), drop_nonver_sql) + )$drop_sql, drop_nonver_sql) }) test_that("user error non-versioned drop sql created correctly", { @@ -48,5 +48,5 @@ test_that("user error non-versioned drop sql created correctly", { expect_equal(create_drop_sql( "test", "test", "test_schema", "test_tbl" - ), drop_nonver_sql) + )$drop_sql, drop_nonver_sql) }) From 9cc55292807a20e738399d7c4b3f38a980edb089 Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:36:47 +0100 Subject: [PATCH 09/11] warning that created system versioned table --- R/create_replace_table.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/R/create_replace_table.R b/R/create_replace_table.R index fa639ea..e32115b 100644 --- a/R/create_replace_table.R +++ b/R/create_replace_table.R @@ -580,6 +580,11 @@ write_dataframe_to_db <- function(server, # Drop the staging table and finished delete_staging_table(db_params) + if (versioned_table) { + warning(glue::glue("Created {schema}.{table_name} as versioned table"), + .call = FALSE + ) + } end_time <- Sys.time() message(glue::glue( "Loading completed in", From ae0873679524765bc63d839e73ecfd54190aa0cf Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 08:44:59 +0100 Subject: [PATCH 10/11] fix warning hide call --- R/create_replace_table.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/create_replace_table.R b/R/create_replace_table.R index e32115b..75cbf4c 100644 --- a/R/create_replace_table.R +++ b/R/create_replace_table.R @@ -582,7 +582,7 @@ write_dataframe_to_db <- function(server, delete_staging_table(db_params) if (versioned_table) { warning(glue::glue("Created {schema}.{table_name} as versioned table"), - .call = FALSE + call. = FALSE ) } end_time <- Sys.time() From e2994e9f179de8e541874c969a3d0ba58247e32e Mon Sep 17 00:00:00 2001 From: tomwilsonsco Date: Tue, 13 Aug 2024 07:54:46 +0000 Subject: [PATCH 11/11] Update documentation --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 761e6af..0324e54 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -17,7 +17,7 @@ Imports: glue Language: en-GB Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.1 +RoxygenNote: 7.3.2 Suggests: testthat (>= 2.1.0), mockery