SQL Server: Handle temporary flag in sqlCreateTable (#601)

* SQL Server: Handle temporary flag in sqlCreateTable

* code-review: better warning + testthat usage

* code-review: simplify sql server specific method

* code-review: Add missing _snaps

* Update news

NEWS.md

@@ -1,4 +1,7 @@
 # odbc (development version)
+
+* SQL Server: Specialize syntax in sqlCreateTable to avoid failures when
+  writing to (new) local temp tables. (@detule, #601)
 * SQL Server: Improved handling for local temp tables in dbWrite, dbAppendTable,
   dbExistTable (@detule, #600)
 * Teradata: Improved handling for temp tables (@detule and @But2ene, #589, 590)

R/db.R

@@ -331,3 +331,24 @@ setMethod(
     }
     NROW(df) > 0
   })
+
+#' SQL Server specific implementation.
+#'
+#' Will warn user if `temporary` is set to TRUE but table name does not conform
+#' to local temp table naming conventions.  If writing to a global temp table, user
+#' should not set the temporary flag to TRUE.
+#'
+#' In both cases a simple CREATE TABLE statement is used / the table identifier
+#' is the differentiator ( viz-a-viz creating a non-temp table ).
+#' @inheritParams DBI::sqlCreateTable
+#' @rdname SQLServer
+#' @usage NULL
+setMethod("sqlCreateTable", "Microsoft SQL Server",
+  function(con, table, fields, row.names = NA, temporary = FALSE, ..., field.types = NULL) {
+    if ( temporary && !isTempTable( con, table ) )
+    {
+      warning("Temporary flag is set to true, but table name doesn't use # prefix")
+    }
+    temporary <- FALSE
+    callNextMethod()
+})

tests/testthat/_snaps/SQLServer.md

@@ -0,0 +1,8 @@
+# Create / write to temp table
+
+    Temporary flag is set to true, but table name doesn't use # prefix
+
+---
+
+    Temporary flag is set to true, but table name doesn't use # prefix
+

tests/testthat/test-SQLServer.R

@@ -262,4 +262,40 @@ test_that("SQLServer", {
     # Fail because table not actually present
     expect_true( !dbExistsTable( con, tbl_name3, catalog_name = "tempdb" ) )
   })
+
+  test_that("Create / write to temp table", {
+    testthat::local_edition(3)
+    con <- DBItest:::connect(DBItest:::get_default_context())
+    locTblName <- "#myloctmp"
+    globTblName <- "##myglobtmp"
+    notTempTblName <- "nottemp"
+
+    df <- data.frame( name = c("one", "two"), value = c(1, 2) )
+    values <- sqlData(con, row.names = FALSE, df[, , drop = FALSE])
+    ret1 <- sqlCreateTable(con, locTblName, values, temporary = TRUE)
+    ret2 <- sqlCreateTable(con, locTblName, values, temporary = FALSE)
+
+    nm <- dbQuoteIdentifier(con, locTblName)
+    fields <- createFields(con, values, row.names = FALSE, field.types = NULL)
+    expected <- DBI::SQL(paste0(
+      "CREATE TABLE ", nm, " (\n",
+      "  ", paste(fields, collapse = ",\n  "), "\n)\n"
+    ))
+
+    expect_equal( ret1, expected )
+    expect_equal( ret2, expected )
+    expect_snapshot_warning(sqlCreateTable(con, globTblName, values, temporary = TRUE))
+    expect_no_warning(sqlCreateTable(con, globTblName, values, temporary = FALSE))
+    expect_snapshot_warning(sqlCreateTable(con, notTempTblName, values, temporary = TRUE))
+    expect_no_warning(sqlCreateTable(con, notTempTblName, values, temporary = FALSE))
+
+    # These tests need https://github.com/r-dbi/odbc/pull/600
+    # Uncomment when both merged.
+    # dbWriteTable(con, locTblName, mtcars, row.names = TRUE)
+    # res <- dbGetQuery(con, paste0("SELECT * FROM ", locTblName))
+    # expect_equal( mtcars$mpg, res$mpg )
+    # dbAppendTable(con, locTblName, mtcars)
+    # res <- dbGetQuery(con, paste0("SELECT * FROM ", locTblName))
+    # expect_equal( nrow( res ), 2 * nrow( mtcars ) )
+  })
 })