Merge pull request #3 from oxford-pharmacoepi/main

update from main

DESCRIPTION

@@ -48,7 +48,7 @@ Suggests:
     rmarkdown,
     RPostgres (>= 1.4.5),
     RPostgreSQL,
-    RSQLite (>= 2.3.1),
+    RSQLite (>= 2.3.8),
     testthat (>= 3.1.10)
 VignetteBuilder: 
     knitr
@@ -58,7 +58,7 @@ Config/testthat/parallel: TRUE
 Encoding: UTF-8
 Language: en-gb
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.3.1
+RoxygenNote: 7.3.2
 Collate: 
     'db-sql.R'
     'utils-check.R'

NEWS.md

@@ -1,10 +1,28 @@
 # dbplyr (development version)
 
+* Tightened argument checks for SQL translations. These changes should 
+  result in more informative errors in cases where code already failed, possibly
+  silently; if you see errors with code that used to run correctly, please report 
+  them to the package authors (@simonpcouch, #1554, #1555).
+
+* `clock::add_years()` translates to correct SQL on Spark (@ablack3, #1510).
+
+* Translations for `as.double()` and `as.character()` with Teradata previously
+  raised errors and are now correct (@rplsmn, #1545).
+
+* Translations of `difftime()` for Postgres, SQL server, Redshift, and Snowflake
+  previously returned the wrong sign and are now correct (@edward-burn, #1532). 
+
 * `across(everything())` doesn't select grouping columns created via `.by` in
   `summarise()` (@mgirlich, #1493).
 *  Use `COUNT_BIG` instead of `COUNT` for SQL server so that `tally()` and
   `count()` work regardless of size of the data (@edward-burn, #1498).
 
+*  New translations of clock function `date_count_between()` for SQL server, Redshift, Snowflake, Postgres, and Spark (@edward-burn, #1495).
+
+* Spark SQL backend now supports persisting tables with
+  `compute(x, name = I("x.y.z"), temporary = FALSE)` (@zacdav-db, #1502).
+
 # dbplyr 2.5.0
 
 ## Improved tools for qualified table names

R/backend-.R

@@ -252,6 +252,7 @@ base_scalar <- sql_translator(
   # base R
   nchar = sql_prefix("LENGTH", 1),
   nzchar = function(x, keepNA = FALSE) {
+    check_bool(keepNA)
     if (keepNA) {
       exp <- expr(!!x != "")
       translate_sql(!!exp, con = sql_current_con())
@@ -281,6 +282,7 @@ base_scalar <- sql_translator(
   str_c = sql_paste(""),
   str_sub = sql_str_sub("SUBSTR"),
   str_like = function(string, pattern, ignore_case = TRUE) {
+    check_bool(ignore_case)
     if (isTRUE(ignore_case)) {
       sql_expr(!!string %LIKE% !!pattern)
     } else {

R/backend-hive.R

@@ -94,6 +94,7 @@ sql_table_analyze.Hive <- function(con, table, ...) {
 
 #' @export
 sql_query_set_op.Hive <- function(con, x, y, method, ..., all = FALSE, lvl = 0) {
+  check_bool(all)
   # parentheses are not allowed
   method <- paste0(method, if (all) " ALL")
   glue_sql2(

R/backend-mssql.R

@@ -22,7 +22,7 @@
 #'
 #' * The `BOOLEAN` type is the result of logical comparisons (e.g. `x > y`)
 #'   and can be used `WHERE` but not to create new columns in `SELECT`.
-#'   <https://docs.microsoft.com/en-us/sql/t-sql/language-elements/comparison-operators-transact-sql>
+#'   <https://learn.microsoft.com/en-us/sql/t-sql/language-elements/comparison-operators-transact-sql>
 #'
 #' * The `BIT` type is a special type of numeric column used to store
 #'   `TRUE` and `FALSE` values, but can't be used in `WHERE` clauses.
@@ -107,11 +107,15 @@ simulate_mssql <- function(version = "15.0") {
                                                     conflict = c("error", "ignore"),
                                                     returning_cols = NULL,
                                                     method = NULL) {
-  method <- method %||% "where_not_exists"
-  arg_match(method, "where_not_exists", error_arg = "method")
   # https://stackoverflow.com/questions/25969/insert-into-values-select-from
   conflict <- rows_check_conflict(conflict)
 
+  check_character(returning_cols, allow_null = TRUE)
+
+  check_string(method, allow_null = TRUE)
+  method <- method %||% "where_not_exists"
+  arg_match(method, "where_not_exists", error_arg = "method")
+
   parts <- rows_insert_prep(con, table, from, insert_cols, by, lvl = 0)
 
   clauses <- list2(
@@ -177,6 +181,7 @@ simulate_mssql <- function(version = "15.0") {
                                                     ...,
                                                     returning_cols = NULL,
                                                     method = NULL) {
+  check_string(method, allow_null = TRUE)
   method <- method %||% "merge"
   arg_match(method, "merge", error_arg = "method")
 
@@ -333,6 +338,7 @@ simulate_mssql <- function(version = "15.0") {
       second = function(x) sql_expr(DATEPART(SECOND, !!x)),
 
       month = function(x, label = FALSE, abbr = TRUE) {
+        check_bool(label)
         if (!label) {
           sql_expr(DATEPART(MONTH, !!x))
         } else {
@@ -342,6 +348,7 @@ simulate_mssql <- function(version = "15.0") {
       },
 
       quarter = function(x, with_year = FALSE, fiscal_start = 1) {
+        check_bool(with_year)
         check_unsupported_arg(fiscal_start, 1, backend = "SQL Server")
 
         if (with_year) {
@@ -361,6 +368,7 @@ simulate_mssql <- function(version = "15.0") {
         sql_expr(DATEADD(YEAR, !!n, !!x))
       },
       date_build = function(year, month = 1L, day = 1L, ..., invalid = NULL) {
+        check_unsupported_arg(invalid, allow_null = TRUE)
         sql_expr(DATEFROMPARTS(!!year, !!month, !!day))
       },
       get_year = function(x) {
@@ -372,18 +380,19 @@ simulate_mssql <- function(version = "15.0") {
       get_day = function(x) {
         sql_expr(DATEPART(DAY, !!x))
       },
+      date_count_between = function(start, end, precision, ..., n = 1L){
+        check_dots_empty()
+        check_unsupported_arg(precision, allowed = "day")
+        check_unsupported_arg(n, allowed = 1L)
 
-      difftime = function(time1, time2, tz, units = "days") {
-
-        if (!missing(tz)) {
-          cli::cli_abort("The {.arg tz} argument is not supported for SQL backends.")
-        }
+        sql_expr(DATEDIFF(DAY, !!start, !!end))
+      },
 
-        if (units[1] != "days") {
-          cli::cli_abort('The only supported value for {.arg units} on SQL backends is "days"')
-        }
+      difftime = function(time1, time2, tz, units = "days") {
+        check_unsupported_arg(tz)
+        check_unsupported_arg(units, allowed = "days")
 
-        sql_expr(DATEDIFF(DAY, !!time1, !!time2))
+        sql_expr(DATEDIFF(DAY, !!time2, !!time1))
       }
     )
 
@@ -534,7 +543,7 @@ mssql_version <- function(con) {
 
 #' @export
 `sql_returning_cols.Microsoft SQL Server` <- function(con, cols, table, ...) {
-  stopifnot(table %in% c("DELETED", "INSERTED"))
+  arg_match(table, values = c("DELETED", "INSERTED"))
   returning_cols <- sql_named_cols(con, cols, table = table)
 
   sql_clause("OUTPUT", returning_cols)

R/backend-oracle.R

@@ -13,7 +13,7 @@
 #'
 #' Note that versions of Oracle prior to 23c have limited supported for
 #' `TRUE` and `FALSE` and you may need to use `1` and `0` instead.
-#' See <https://oracle-base.com/articles/23c/boolean-data-type-23c> for
+#' See <https://oracle-base.com/articles/23/boolean-data-type-23> for
 #' more details.
 #'
 #' Use `simulate_oracle()` with `lazy_frame()` to see simulated SQL without

R/backend-postgres.R

@@ -57,6 +57,7 @@ postgres_grepl <- function(pattern,
   check_unsupported_arg(perl, FALSE, backend = "PostgreSQL")
   check_unsupported_arg(fixed, FALSE, backend = "PostgreSQL")
   check_unsupported_arg(useBytes, FALSE, backend = "PostgreSQL")
+  check_bool(ignore.case)
 
   if (ignore.case) {
     sql_expr(((!!x)) %~*% ((!!pattern)))
@@ -123,6 +124,7 @@ sql_translation.PqConnection <- function(con) {
       },
       # https://www.postgresql.org/docs/current/functions-matching.html
       str_like = function(string, pattern, ignore_case = TRUE) {
+        check_bool(ignore_case)
         if (isTRUE(ignore_case)) {
           sql_expr(!!string %ILIKE% !!pattern)
         } else {
@@ -162,6 +164,9 @@ sql_translation.PqConnection <- function(con) {
         sql_expr(EXTRACT(DAY %FROM% !!x))
       },
       wday = function(x, label = FALSE, abbr = TRUE, week_start = NULL) {
+        check_bool(label)
+        check_bool(abbr)
+        check_number_whole(week_start, allow_null = TRUE)
         if (!label) {
           week_start <- week_start %||% getOption("lubridate.week.start", 7)
           offset <- as.integer(7 - week_start)
@@ -182,6 +187,8 @@ sql_translation.PqConnection <- function(con) {
         sql_expr(EXTRACT(WEEK %FROM% !!x))
       },
       month = function(x, label = FALSE, abbr = TRUE) {
+        check_bool(label)
+        check_bool(abbr)
         if (!label) {
           sql_expr(EXTRACT(MONTH %FROM% !!x))
         } else {
@@ -193,6 +200,7 @@ sql_translation.PqConnection <- function(con) {
         }
       },
       quarter = function(x, with_year = FALSE, fiscal_start = 1) {
+        check_bool(with_year)
         check_unsupported_arg(fiscal_start, 1, backend = "PostgreSQL")
 
         if (with_year) {
@@ -246,8 +254,17 @@ sql_translation.PqConnection <- function(con) {
         glue_sql2(sql_current_con(), "({.col x} + {.val n}*INTERVAL'1 year')")
       },
       date_build = function(year, month = 1L, day = 1L, ..., invalid = NULL) {
+        check_unsupported_arg(invalid, allow_null = TRUE)
         sql_expr(make_date(!!year, !!month, !!day))
       },
+      date_count_between = function(start, end, precision, ..., n = 1L){
+
+        check_dots_empty()
+        check_unsupported_arg(precision, allowed = "day")
+        check_unsupported_arg(n, allowed = 1L)
+
+        sql_expr(!!end - !!start)
+      },
       get_year = function(x) {
         sql_expr(date_part('year', !!x))
       },
@@ -260,15 +277,10 @@ sql_translation.PqConnection <- function(con) {
 
       difftime = function(time1, time2, tz, units = "days") {
 
-        if (!missing(tz)) {
-          cli::cli_abort("The {.arg tz} argument is not supported for SQL backends.")
-        }
-
-        if (units[1] != "days") {
-          cli::cli_abort('The only supported value for {.arg units} on SQL backends is "days"')
-        }
+        check_unsupported_arg(tz)
+        check_unsupported_arg(units, allowed = "days")
 
-        sql_expr((CAST(!!time2 %AS% DATE) - CAST(!!time1 %AS% DATE)))
+        sql_expr((CAST(!!time1 %AS% DATE) - CAST(!!time2 %AS% DATE)))
       },
     ),
     sql_translator(.parent = base_agg,
@@ -332,6 +344,7 @@ sql_query_insert.PqConnection <- function(con,
                                           ...,
                                           returning_cols = NULL,
                                           method = NULL) {
+  check_string(method, allow_null = TRUE)
   method <- method %||% "on_conflict"
   arg_match(method, c("on_conflict", "where_not_exists"), error_arg = "method")
   if (method == "where_not_exists") {
@@ -367,6 +380,7 @@ sql_query_upsert.PqConnection <- function(con,
                                           ...,
                                           returning_cols = NULL,
                                           method = NULL) {
+  check_string(method, allow_null = TRUE)
   method <- method %||% "on_conflict"
   arg_match(method, c("cte_update", "on_conflict"), error_arg = "method")
 

R/backend-redshift.R

@@ -72,6 +72,7 @@ sql_translation.RedshiftConnection <- function(con) {
         sql_expr(DATEADD(YEAR, !!n, !!x))
       },
       date_build = function(year, month = 1L, day = 1L, ..., invalid = NULL) {
+        check_unsupported_arg(invalid, allow_null = TRUE)
         glue_sql2(sql_current_con(), "TO_DATE(CAST({.val year} AS TEXT) || '-' CAST({.val month} AS TEXT) || '-' || CAST({.val day} AS TEXT)), 'YYYY-MM-DD')")
       },
       get_year = function(x) {
@@ -83,18 +84,19 @@ sql_translation.RedshiftConnection <- function(con) {
       get_day = function(x) {
         sql_expr(DATE_PART('day', !!x))
       },
+      date_count_between = function(start, end, precision, ..., n = 1L){
+        check_dots_empty()
+        check_unsupported_arg(precision, allowed = "day")
+        check_unsupported_arg(n, allowed = 1L)
 
-      difftime = function(time1, time2, tz, units = "days") {
-
-        if (!missing(tz)) {
-          cli::cli_abort("The {.arg tz} argument is not supported for SQL backends.")
-        }
+        sql_expr(DATEDIFF(DAY, !!start, !!end))
+      },
 
-        if (units[1] != "days") {
-          cli::cli_abort('The only supported value for {.arg units} on SQL backends is "days"')
-        }
+      difftime = function(time1, time2, tz, units = "days") {
+        check_unsupported_arg(tz)
+        check_unsupported_arg(units, allowed = "days")
 
-        sql_expr(DATEDIFF(DAY, !!time1, !!time2))
+        sql_expr(DATEDIFF(DAY, !!time2, !!time1))
       }
     ),
     sql_translator(.parent = postgres$aggregate,