diff --git a/DESCRIPTION b/DESCRIPTION index 3a0d96337..63f6b55f8 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -18,6 +18,7 @@ Depends: Imports: blob (>= 1.2.0), callr, + constructive, DBI (>= 1.1.3.9004), desc, hms (>= 0.5.0), diff --git a/R/spec-meta-bind-.R b/R/spec-meta-bind-.R index 1dd4ab0ad..16b9dd14a 100644 --- a/R/spec-meta-bind-.R +++ b/R/spec-meta-bind-.R @@ -1,30 +1,48 @@ # Helpers ----------------------------------------------------------------- -test_select_bind <- function(con, ctx, values, ...) { +test_select_bind <- function(con, ctx, bind_values, ..., requires_names = NULL) { + placeholder_funs <- get_placeholder_funs(ctx, requires_names) + lapply( - get_placeholder_funs(ctx), + placeholder_funs, test_select_bind_one, con = con, - values = values, + bind_values = bind_values, is_null_check = ctx$tweaks$is_null_check, allow_na_rows_affected = ctx$tweaks$allow_na_rows_affected, ... ) } -get_placeholder_funs <- function(ctx) { +get_placeholder_funs <- function(ctx, requires_names = NULL) { placeholder_fun <- ctx$tweaks$placeholder_pattern if (is.character(placeholder_fun)) { - placeholder_fun <- lapply(placeholder_fun, make_placeholder_fun) + placeholder_funs <- lapply(placeholder_fun, make_placeholder_fun) } else if (is.function(placeholder_fun)) { - placeholder_fun <- list(placeholder_fun) + placeholder_funs <- list(placeholder_fun) + } else { + placeholder_funs <- placeholder_fun } - if (length(placeholder_fun) == 0) { + if (length(placeholder_funs) == 0) { skip("Use the placeholder_pattern tweak, or skip all 'bind_.*' tests") } - placeholder_fun + if (!is.null(requires_names)) { + placeholder_fun_values <- map(placeholder_funs, ~ .x(1)) + placeholder_unnamed <- map_lgl(placeholder_fun_values, ~ is.null(names(.x))) + + # run_bind_tester$fun() + if (isTRUE(requires_names)) { + placeholder_funs <- placeholder_funs[!placeholder_unnamed] + } + + if (isFALSE(requires_names)) { + placeholder_funs <- placeholder_funs[placeholder_unnamed] + } + } + + placeholder_funs } test_select_bind_one <- function( @@ -36,12 +54,12 @@ test_select_bind_one <- function( cast_fun = identity, allow_na_rows_affected = FALSE, # Spec time - values, + bind_values, query = TRUE, + skip_fun = NULL, check_return_value = NULL, - patch_bind_values = identity, + patch_bind_values = NULL, bind_error = NA, - requires_names = NULL, is_repeated = FALSE, is_premature_clear = FALSE, is_untouched = FALSE) { @@ -54,12 +72,12 @@ test_select_bind_one <- function( is_null_check = is_null_check, cast_fun = cast_fun, allow_na_rows_affected = allow_na_rows_affected, - values = values, + bind_values = bind_values, query = query, + skip_fun = skip_fun, check_return_value = check_return_value, patch_bind_values = patch_bind_values, bind_error = bind_error, - requires_names = requires_names, is_repeated = is_repeated, is_premature_clear = is_premature_clear, is_untouched = is_untouched diff --git a/R/spec-meta-bind-runner.R b/R/spec-meta-bind-runner.R index aa2691f87..85a3d9d40 100644 --- a/R/spec-meta-bind-runner.R +++ b/R/spec-meta-bind-runner.R @@ -18,12 +18,12 @@ run_bind_tester$fun <- function( cast_fun, allow_na_rows_affected, # Spec time - values, + bind_values, query, + skip_fun, check_return_value, patch_bind_values, bind_error, - requires_names, is_repeated, is_premature_clear, is_untouched) { @@ -32,29 +32,41 @@ run_bind_tester$fun <- function( force(is_null_check) force(cast_fun) force(allow_na_rows_affected) - force(values) + force(bind_values) force(query) + force(skip) force(check_return_value) force(patch_bind_values) force(bind_error) - force(requires_names) force(is_repeated) force(is_premature_clear) force(is_untouched) - # From R6 class - is_query <- function() { - query + if (is.null(patch_bind_values)) { + patch_bind_values <- identity } - # - send_query <- function() { + + bind_values_expr <- rlang::expr({ + bind_values <- !!construct_expr(bind_values) + }) + + skip_expr <- if (!is.null(skip_fun) && skip_fun()) rlang::expr({ + skip(rlang::expr_deparse(body(skip_fun))) + }) + + #' 1. Call [dbSendQuery()] or [dbSendStatement()] with a query or statement + #' that contains placeholders, + #' store the returned [DBIResult-class] object in a variable. + #' Mixing placeholders (in particular, named and unnamed ones) is not + #' recommended. + send_expr <- if (query) rlang::expr({ ret_values <- trivial_values(2) - placeholder <- placeholder_fun(length(values)) - is_na <- vapply(values, is_na_or_null, logical(1)) - placeholder_values <- vapply(values, function(x) DBI::dbQuoteLiteral(con, x[1]), character(1)) - result_names <- letters[seq_along(values)] + placeholder <- placeholder_fun(length(bind_values)) + is_na <- vapply(bind_values, is_na_or_null, logical(1)) + placeholder_values <- vapply(bind_values, function(x) DBI::dbQuoteLiteral(con, x[1]), character(1)) + result_names <- letters[seq_along(bind_values)] - query <- paste0( + sql <- paste0( "SELECT ", paste0( "CASE WHEN ", @@ -70,153 +82,152 @@ run_bind_tester$fun <- function( ) ) - dbSendQuery(con, query) - } - # - send_statement <- function() { + res <- dbSendQuery(con, sql) + }) else rlang::expr({ data <- data.frame(a = rep(1:5, 1:5)) data$b <- seq_along(data$a) table_name <- random_table_name() dbWriteTable(con, table_name, data, temporary = TRUE) - value_names <- letters[seq_along(values)] - placeholder <- placeholder_fun(length(values)) - statement <- paste0( + value_names <- letters[seq_along(bind_values)] + placeholder <- placeholder_fun(length(bind_values)) + sql <- paste0( "UPDATE ", dbQuoteIdentifier(con, table_name), " SET b = b + 1 WHERE ", paste(value_names, " = ", placeholder, collapse = " AND ") ) - dbSendStatement(con, statement) - } - # - bind <- function(res, bind_values) { - bind_values <- patch_bind_values(bind_values) - expect_error(bind_res <- withVisible(dbBind(res, bind_values)), bind_error) - - if (!is.null(check_return_value) && is.na(bind_error) && exists("bind_res")) { - check_return_value(bind_res, res) - } - invisible() - } - # - compare <- function(rows) { - expect_equal(nrow(rows), length(values[[1]])) - if (nrow(rows) > 0) { - result_names <- letters[seq_along(values)] - expected <- c(trivial_values(1), rep(trivial_values(2)[[2]], nrow(rows) - 1)) - all_expected <- rep(list(expected), length(values)) - result <- as.data.frame(setNames(all_expected, result_names)) - - expect_equal(rows, result) - } - } - # - compare_affected <- function(rows_affected, values) { - # Allow NA value for dbGetRowsAffected(), #297 - if (isTRUE(allow_na_rows_affected) && is.na(rows_affected)) { - return() - } - expect_equal(rows_affected, sum(values[[1]])) - } - - # run_bind_tester$fun() - if (isTRUE(requires_names) && is.null(names(placeholder_fun(1)))) { - # test only valid for named placeholders - return() - } - - if (isFALSE(requires_names) && !is.null(names(placeholder_fun(1)))) { - # test only valid for unnamed placeholders - return() - } - - #' 1. Call [dbSendQuery()] or [dbSendStatement()] with a query or statement - #' that contains placeholders, - #' store the returned [DBIResult-class] object in a variable. - #' Mixing placeholders (in particular, named and unnamed ones) is not - #' recommended. - if (is_query()) { - res <- send_query() - } else { - res <- send_statement() - } + res <- dbSendStatement(con, sql) + }) #' It is good practice to register a call to [dbClearResult()] via #' [on.exit()] right after calling `dbSendQuery()` or `dbSendStatement()` #' (see the last enumeration item). - if (is_premature_clear) { + clear_expr <- if (is_premature_clear) rlang::expr({ dbClearResult(res) - } else { - on.exit(expect_error(dbClearResult(res), NA)) + }) else { + on_exit_expr <- rlang::expr({ + on.exit(expect_error(dbClearResult(res), NA)) + }) + + #' Until `dbBind()` has been called, the returned result set object has the + #' following behavior: + initial_state_expr <- if (query) rlang::expr({ + #' - [dbFetch()] raises an error (for `dbSendQuery()`) + expect_error(dbFetch(res)) + #' - [dbGetRowCount()] returns zero (for `dbSendQuery()`) + expect_equal(dbGetRowCount(res), 0) + }) else rlang::expr({ + #' - [dbGetRowsAffected()] returns an integer `NA` (for `dbSendStatement()`) + expect_identical(dbGetRowsAffected(res), NA_integer_) + }) + + initial_state_expr_2 <- rlang::expr({ + #' - [dbIsValid()] returns `TRUE` + expect_true(dbIsValid(res)) + #' - [dbHasCompleted()] returns `FALSE` + expect_false(dbHasCompleted(res)) + }) + + rlang::expr({ + !!!on_exit_expr + !!!initial_state_expr + !!!initial_state_expr_2 + }) } - #' Until `dbBind()` has been called, the returned result set object has the - #' following behavior: - #' - [dbFetch()] raises an error (for `dbSendQuery()`) - if (is_query()) expect_error(dbFetch(res)) - #' - [dbGetRowCount()] returns zero (for `dbSendQuery()`) - if (is_query()) expect_equal(dbGetRowCount(res), 0) - #' - [dbGetRowsAffected()] returns an integer `NA` (for `dbSendStatement()`) - if (!is_query()) expect_identical(dbGetRowsAffected(res), NA_integer_) - #' - [dbIsValid()] returns `TRUE` - expect_true(dbIsValid(res)) - #' - [dbHasCompleted()] returns `FALSE` - expect_false(dbHasCompleted(res)) - #' 1. Construct a list with parameters #' that specify actual values for the placeholders. - bind_values <- values #' The list must be named or unnamed, #' depending on the kind of placeholders used. #' Named values are matched to named parameters, unnamed values #' are matched by position in the list of parameters. - - if (!is.null(names(placeholder_fun(1)))) { + name_values_expr <- rlang::expr(if (!is.null(names(placeholder_fun(1)))) { names(bind_values) <- names(placeholder_fun(length(bind_values))) - } + }) + #' All elements in this list must have the same lengths and contain values #' supported by the backend; a [data.frame] is internally stored as such #' a list. #' The parameter list is passed to a call to `dbBind()` on the `DBIResult` #' object. - bind(res, bind_values) - if (!is.na(bind_error)) { - return() - } - - # Safety net: returning early if dbBind() should have thrown an error but - # didn't - if (!identical(bind_values, patch_bind_values(bind_values))) { - return() - } - if (is_premature_clear) { - return() - } + bind_expr <- if (is.na(bind_error)) rlang::expr({ + bind_res <- withVisible(dbBind(res, patch_bind_values(bind_values))) + if (!is.null(check_return_value)) { + check_return_value(bind_res, res) + } + }) else rlang::expr({ + expect_error( + withVisible(dbBind(res, patch_bind_values(bind_values))), + bind_error + ) + }) #' 1. Retrieve the data or the number of affected rows from the `DBIResult` object. - retrieve <- function() { - #' - For queries issued by `dbSendQuery()`, - #' call [dbFetch()]. - if (is_query()) { - rows <- check_df(dbFetch(res)) - compare(rows) - } else { - #' - For statements issued by `dbSendStatements()`, - #' call [dbGetRowsAffected()]. - #' (Execution begins immediately after the `dbBind()` call, - #' the statement is processed entirely before the function returns.) - rows_affected <- dbGetRowsAffected(res) - compare_affected(rows_affected, values) + #' - For queries issued by `dbSendQuery()`, + #' call [dbFetch()]. + retrieve_expr <- if (query) rlang::expr({ + rows <- check_df(dbFetch(res)) + expect_equal(nrow(rows), length(bind_values[[1]])) + if (nrow(rows) > 0) { + result_names <- letters[seq_along(bind_values)] + expected <- c(trivial_values(1), rep(trivial_values(2)[[2]], nrow(rows) - 1)) + all_expected <- rep(list(expected), length(bind_values)) + result <- as.data.frame(setNames(all_expected, result_names)) + + expect_equal(rows, result) } - } + }) else rlang::expr({ + #' - For statements issued by `dbSendStatements()`, + #' call [dbGetRowsAffected()]. + #' (Execution begins immediately after the `dbBind()` call, + #' the statement is processed entirely before the function returns.) + rows_affected <- dbGetRowsAffected(res) + # Allow NA value for dbGetRowsAffected(), #297 + if (!isTRUE(allow_na_rows_affected) || !is.na(rows_affected)) { + expect_equal(rows_affected, sum(bind_values[[1]])) + } + }) - if (!is_untouched) retrieve() + not_untouched_expr <- if (!is_untouched) rlang::expr({ + !!retrieve_expr + }) #' 1. Repeat 2. and 3. as necessary. - if (is_repeated) { - bind(res, bind_values) - retrieve() - } + repeated_expr <- if (is_repeated) rlang::expr({ + bind_res <- withVisible(dbBind(res, patch_bind_values(bind_values))) + if (!is.null(check_return_value)) { + check_return_value(bind_res, res) + } + !!retrieve_expr + }) #' 1. Close the result set via [dbClearResult()]. + + early_exit <- + is_premature_clear || + !is.na(bind_error) || + !identical(bind_values, patch_bind_values(bind_values)) + + post_bind_expr <- if (!early_exit) rlang::expr({ + !!not_untouched_expr + !!repeated_expr + }) + + test_expr <- rlang::expr({ + !!bind_values_expr + !!skip_expr + !!send_expr + !!clear_expr + !!name_values_expr + !!bind_expr + !!post_bind_expr + }) + + rm(bind_values) + rlang::eval_bare(test_expr) +} + +construct_expr <- function(x) { + xc <- constructive::construct(x) + rlang::parse_expr(format(xc$code)) } diff --git a/R/spec-meta-bind.R b/R/spec-meta-bind.R index 0c7d8498d..66ea7b839 100644 --- a/R/spec-meta-bind.R +++ b/R/spec-meta-bind.R @@ -8,7 +8,7 @@ spec_meta_bind <- list( # expect_equal(names(formals(dbBind)), c("res", "params", "...")) }, - + # bind_return_value = function(ctx, con) { #' @return check_return_value <- function(bind_res, res) { @@ -25,6 +25,14 @@ spec_meta_bind <- list( 1L, check_return_value = check_return_value ) + }, + # + bind_return_value_statement = function(ctx, con) { + check_return_value <- function(bind_res, res) { + expect_identical(res, bind_res$value) + expect_false(bind_res$visible) + } + #' and also for data manipulation statements issued by #' [dbSendStatement()]. test_select_bind( @@ -107,7 +115,7 @@ spec_meta_bind <- list( query = FALSE ) }, - + # bind_named_param_unnamed_placeholders = function(ctx, con) { #' If the placeholders in the query are named, patch_bind_values <- function(bind_values) { @@ -141,7 +149,7 @@ spec_meta_bind <- list( }, # bind_named_param_na_placeholders = function(ctx, con) { - patch_bind_values = function(bind_values) { + patch_bind_values <- function(bind_values) { #' or `NA`), names(bind_values)[[1]] <- NA bind_values @@ -180,9 +188,13 @@ spec_meta_bind <- list( bind_premature_clear = function(ctx, con) { #' Calling `dbBind()` on a result set already cleared by [dbClearResult()] is_premature_clear <- TRUE - expect_error( - #' also raises an error. - test_select_bind(con, ctx, 1L, is_premature_clear = is_premature_clear) + #' also raises an error. + test_select_bind( + con, + ctx, + 1L, + is_premature_clear = is_premature_clear, + bind_error = ".*" ) }, @@ -217,6 +229,11 @@ spec_meta_bind <- list( #' for both queries test_select_bind(con, ctx, 1L, is_repeated = is_repeated) + }, + # + bind_repeated_statement = function(ctx, con) { + is_repeated <- TRUE + #' and data manipulation statements, test_select_bind(con, ctx, 1L, is_repeated = is_repeated, query = FALSE) }, @@ -234,6 +251,12 @@ spec_meta_bind <- list( is_repeated = is_repeated, is_untouched = is_untouched ) + }, + # + bind_repeated_untouched_statement = function(ctx, con) { + is_repeated <- TRUE + is_untouched <- TRUE + #' and data manipulation statements. test_select_bind( con, @@ -283,9 +306,13 @@ spec_meta_bind <- list( test_select_bind(con, ctx, c(get_texts(), NA)) }, - bind_character_escape = function(ctx, con) { + bind_character_escape = if (getRversion() >= "4.0") function(ctx, con) { #' (also with special characters such as spaces, newlines, quotes, and backslashes) - test_select_bind(con, ctx, c(" ", "\n", "\r", "\b", "'", '"', "[", "]", "\\", NA)) + test_select_bind( + con, + ctx, + c(" ", "\n", "\r", "\b", "'", '"', "[", "]", "\\", NA) + ) }, bind_factor = function(ctx, con) { @@ -302,97 +329,100 @@ spec_meta_bind <- list( bind_date = function(ctx, con) { #' - [Date] - if (!isTRUE(ctx$tweaks$date_typed)) { - skip("tweak: !date_typed") - } - - test_select_bind(con, ctx, c(Sys.Date() + 0:2, NA)) + test_select_bind( + con, + ctx, + c(Sys.Date() + 0:2, NA), + skip_fun = function() !isTRUE(ctx$tweaks$date_typed) + ) }, bind_date_integer = function(ctx, con) { #' (also when stored internally as integer) - if (!isTRUE(ctx$tweaks$date_typed)) { - skip("tweak: !date_typed") - } - - test_select_bind(con, ctx, structure(c(18618:18620, NA), class = "Date")) + test_select_bind( + con, + ctx, + structure(c(18618:18620, NA), class = "Date"), + skip_fun = function() !isTRUE(ctx$tweaks$date_typed) + ) }, bind_timestamp = function(ctx, con) { #' - [POSIXct] timestamps - if (!isTRUE(ctx$tweaks$timestamp_typed)) { - skip("tweak: !timestamp_typed") - } - data_in <- as.POSIXct(c(round(Sys.time()) + 0:2, NA)) - test_select_bind(con, ctx, data_in) + test_select_bind( + con, + ctx, + data_in, + skip_fun = function() !isTRUE(ctx$tweaks$timestamp_typed) + ) }, bind_timestamp_lt = function(ctx, con) { #' - [POSIXlt] timestamps - if (!isTRUE(ctx$tweaks$timestamp_typed)) { - skip("tweak: !timestamp_typed") - } - data_in <- lapply( round(Sys.time()) + c(0:2, NA), as.POSIXlt ) - test_select_bind(con, ctx, data_in) + test_select_bind( + con, + ctx, + data_in, + skip_fun = function() !isTRUE(ctx$tweaks$timestamp_typed) + ) }, bind_time_seconds = function(ctx, con) { #' - [difftime] values - if (!isTRUE(ctx$tweaks$time_typed)) { - skip("tweak: !time_typed") - } - data_in <- as.difftime(as.numeric(c(1:3, NA)), units = "secs") - test_select_bind(con, ctx, data_in) + test_select_bind( + con, + ctx, + data_in, + skip_fun = function() !isTRUE(ctx$tweaks$time_typed) + ) }, bind_time_hours = function(ctx, con) { #' (also with units other than seconds - if (!isTRUE(ctx$tweaks$time_typed)) { - skip("tweak: !time_typed") - } - data_in <- as.difftime(as.numeric(c(1:3, NA)), units = "hours") - test_select_bind(con, ctx, data_in) + test_select_bind( + con, + ctx, + data_in, + skip_fun = function() !isTRUE(ctx$tweaks$time_typed) + ) }, bind_time_minutes_integer = function(ctx, con) { #' and with the value stored as integer) - if (!isTRUE(ctx$tweaks$time_typed)) { - skip("tweak: !time_typed") - } - data_in <- as.difftime(c(1:3, NA), units = "mins") - test_select_bind(con, ctx, data_in) + test_select_bind( + con, + ctx, + data_in, + skip_fun = function() !isTRUE(ctx$tweaks$time_typed) + ) }, bind_raw = function(ctx, con) { #' - lists of [raw] for blobs (with `NULL` entries for SQL NULL values) - if (isTRUE(ctx$tweaks$omit_blob_tests)) { - skip("tweak: omit_blob_tests") - } - test_select_bind( - con, ctx, + con, + ctx, list(list(as.raw(1:10)), list(raw(3)), list(NULL)), + skip_fun = function() isTRUE(ctx$tweaks$omit_blob_tests), cast_fun = ctx$tweaks$blob_cast ) }, bind_blob = function(ctx, con) { #' - objects of type [blob::blob] - if (isTRUE(ctx$tweaks$omit_blob_tests)) { - skip("tweak: omit_blob_tests") - } - test_select_bind( - con, ctx, + con, + ctx, list(blob::blob(as.raw(1:10)), blob::blob(raw(3)), blob::blob(NULL)), + skip_fun = function() isTRUE(ctx$tweaks$omit_blob_tests), cast_fun = ctx$tweaks$blob_cast ) },