Rethink how dplyr is rendered to sql when using DatabaseConnector

.github/workflows/R_CMD_check_Hades.yaml

@@ -180,4 +180,3 @@ jobs:
         if: ${{ env.new_version != '' }}
         run: |
           curl --data "build=true" -X POST https://registry.hub.docker.com/u/ohdsi/broadsea-methodslibrary/trigger/f0b51cec-4027-4781-9383-4b38b42dd4f5/
-

.github/workflows/R_CMD_check_main_weekly.yaml

@@ -71,4 +71,4 @@ jobs:
         with:
           args: 'c("--no-manual", "--as-cran")'
           error-on: '"warning"'
-          check-dir: '"check"'
+          check-dir: '"check"'

R/Connect.R

@@ -778,7 +778,7 @@
       abort(paste0("Unable to connect JDBC to ", url, " (", rJava::.jcall(x, "S", "getMessage"), ")"))
     }
   }
-  ensureDatabaseConnectorConnectionClassExists()
+  ensureDatabaseConnectorConnectionClassExists(dbms)
   class <- getClassDef("DatabaseConnectorJdbcConnection", where = class_cache, inherits = FALSE)
   if (is.null(class) || methods::isVirtualClass(class)) {
     setClass("DatabaseConnectorJdbcConnection",
@@ -798,16 +798,30 @@
   return(connection)
 }
 
-ensureDatabaseConnectorConnectionClassExists <- function() {
-  class <- getClassDef("Microsoft SQL Server", where = class_cache, inherits = FALSE)
+ensureDatabaseConnectorConnectionClassExists <- function(dbms) {
+  checkIfDbmsIsSupported(dbms)
+  dbmsClass <-  switch(dbms,
+                       "oracle" = "Oracle",
+                       "postgresql" = "PqConnection",
+                       "redshift" = "RedshiftConnection",
+                       "sql server" = "Microsoft SQL Server",
+                       "bigquery" = "BigQueryConnection",
+                       "sqlite" = "SQLiteConnection",
+                       "sqlite extended" = "SQLiteConnection",
+                       "spark" = "Spark SQL",
+                       "snowflake" = "Snowflake",
+                       "synapse" = "Microsoft SQL Server",
+                       "duckdb" = "duckdb_connection",
+                       "blah")
+
+  class <- getClassDef(dbmsClass, where = class_cache, inherits = FALSE)
   if (is.null(class) || methods::isVirtualClass(class)) {
-    setClass("Microsoft SQL Server",
-             where = class_cache)
+    setClass(dbmsClass, where = class_cache)
   }
   class <- getClassDef("DatabaseConnectorConnection", where = class_cache, inherits = FALSE)
   if (is.null(class) || methods::isVirtualClass(class)) {
     setClass("DatabaseConnectorConnection", 
-             contains = c("Microsoft SQL Server", "DBIConnection"),
+             contains = c(dbmsClass, "DBIConnection"),
              slots = list(
                identifierQuote = "character",
                stringQuote = "character",
@@ -822,7 +836,7 @@
   dbms <- dbiConnectionDetails$dbms
   dbiConnectionDetails$dbms <- NULL
   dbiConnection <- do.call(DBI::dbConnect, dbiConnectionDetails)
-  ensureDatabaseConnectorConnectionClassExists()
+  ensureDatabaseConnectorConnectionClassExists(dbms)
   class <- getClassDef("DatabaseConnectorDbiConnection", where = class_cache, inherits = FALSE)
   if (is.null(class) || methods::isVirtualClass(class)) {
     setClass("DatabaseConnectorDbiConnection",
@@ -956,7 +970,10 @@
     "RedshiftConnection" = "redshift",
     "BigQueryConnection" = "bigquery",
     "SQLiteConnection" = "sqlite",
-    "duckdb_connection" = "duckdb"
+    "duckdb_connection" = "duckdb",
+    "Spark Sql" = "spark",
+    "Snowflake" = "snowflake",
+    "Oracle" = "oracle"
     # add mappings from various DBI connection classes to SqlRender dbms here
   )
 }

R/DBI.R

@@ -208,7 +208,7 @@ setClass("DatabaseConnectorDbiResult",
 setMethod(
   "dbSendQuery",
   signature("DatabaseConnectorJdbcConnection", "character"),
-  function(conn, statement, translate = TRUE, ...) {
+  function(conn, statement, translate = FALSE, ...) {
     if (rJava::is.jnull(conn@jConnection)) {
       abort("Connection is closed")
     }
@@ -258,7 +258,7 @@ setMethod(
 setMethod(
   "dbSendQuery",
   signature("DatabaseConnectorDbiConnection", "character"),
-  function(conn, statement, translate = TRUE, ...) {
+  function(conn, statement, translate = FALSE, ...) {
     if (translate) {
       statement <- translateStatement(
         sql = statement,
@@ -426,7 +426,7 @@ setMethod("dbGetRowsAffected", "DatabaseConnectorDbiResult", function(res, ...)
 setMethod(
   "dbGetQuery",
   signature("DatabaseConnectorConnection", "character"),
-  function(conn, statement, translate = TRUE, ...) {
+  function(conn, statement, translate = FALSE, ...) {
     if (translate) {
       statement <- translateStatement(
         sql = statement,
@@ -447,7 +447,7 @@ setMethod(
 setMethod(
   "dbSendStatement",
   signature("DatabaseConnectorConnection", "character"),
-  function(conn, statement, translate = TRUE, ...) {
+  function(conn, statement, translate = FALSE, ...) {
     if (translate) {
       statement <- translateStatement(
         sql = statement,
@@ -472,7 +472,7 @@ setMethod(
 setMethod(
   "dbExecute",
   signature("DatabaseConnectorConnection", "character"),
-  function(conn, statement, translate = TRUE, ...) {
+  function(conn, statement, translate = FALSE, ...) {
     if (isDbplyrSql(statement) && dbms(conn) %in% c("oracle", "bigquery", "spark", "hive") && grepl("^UPDATE STATISTICS", statement)) {
       # These platforms don't support this, so SqlRender translates to an empty string, which causes errors down the line.
       return(0)
@@ -544,7 +544,9 @@ setMethod(
 #' @export
 setMethod(
   "dbWriteTable",
-  "DatabaseConnectorConnection",
+  c(conn = "DatabaseConnectorConnection",
+    name = "character",
+    value = "data.frame"),
   function(conn,
            name, value, databaseSchema = NULL, overwrite = FALSE, append = FALSE, temporary = FALSE, oracleTempSchema = NULL, tempEmulationSchema = getOption("sqlRenderTempEmulationSchema"), ...) {
     if (!is.null(oracleTempSchema) && oracleTempSchema != "") {