diff --git a/NEWS.md b/NEWS.md index 40d67881..e810483c 100644 --- a/NEWS.md +++ b/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) diff --git a/R/db.R b/R/db.R index 8959a3ca..addb3eeb 100644 --- a/R/db.R +++ b/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() +}) diff --git a/man/SQLServer.Rd b/man/SQLServer.Rd index e80d7d07..1e07c4b9 100644 --- a/man/SQLServer.Rd +++ b/man/SQLServer.Rd @@ -7,6 +7,7 @@ \alias{isTempTable,Microsoft SQL Server,character-method} \alias{dbExistsTable,Microsoft SQL Server,character-method} \alias{dbExistsTable} +\alias{sqlCreateTable,Microsoft SQL Server-method} \title{Simple class prototype to avoid messages about unknown classes from setMethod} \arguments{ \item{conn}{A \linkS4class{DBIConnection} object, as returned by @@ -25,6 +26,40 @@ e.g. \code{Id(schema = "my_schema", table = "table_name")} \item a call to \code{\link[DBI:SQL]{SQL()}} with the quoted and fully qualified table name given verbatim, e.g. \code{SQL('"my_schema"."table_name"')} }} + +\item{con}{A database connection.} + +\item{table}{The table name, passed on to \code{\link[DBI:dbQuoteIdentifier]{dbQuoteIdentifier()}}. Options are: +\itemize{ +\item a character string with the unquoted DBMS table name, +e.g. \code{"table_name"}, +\item a call to \code{\link[DBI:Id]{Id()}} with components to the fully qualified table name, +e.g. \code{Id(schema = "my_schema", table = "table_name")} +\item a call to \code{\link[DBI:SQL]{SQL()}} with the quoted and fully qualified table name +given verbatim, e.g. \code{SQL('"my_schema"."table_name"')} +}} + +\item{fields}{Either a character vector or a data frame. + +A named character vector: Names are column names, values are types. +Names are escaped with \code{\link[DBI:dbQuoteIdentifier]{dbQuoteIdentifier()}}. +Field types are unescaped. + +A data frame: field types are generated using +\code{\link[DBI:dbDataType]{dbDataType()}}.} + +\item{row.names}{Either \code{TRUE}, \code{FALSE}, \code{NA} or a string. + +If \code{TRUE}, always translate row names to a column called "row_names". +If \code{FALSE}, never translate row names. If \code{NA}, translate +rownames only if they're a character vector. + +A string is equivalent to \code{TRUE}, but allows you to override the +default name. + +For backward compatibility, \code{NULL} is equivalent to \code{FALSE}.} + +\item{temporary}{If \code{TRUE}, will generate a temporary table statement.} } \value{ \code{dbExistsTable()} returns a logical scalar, \code{TRUE} if the table or view @@ -33,7 +68,7 @@ specified by the \code{name} argument exists, \code{FALSE} otherwise. This includes temporary tables if supported by the database. } \description{ -For SQL Server, conn@quote will return the quotation mark, however +For SQL Server, \code{conn@quote} will return the quotation mark, however both quotation marks as well as square bracket are used interchangeably for delimited identifiers. See: \url{https://learn.microsoft.com/en-us/sql/relational-databases/databases/database-identifiers?view=sql-server-ver16} @@ -47,6 +82,10 @@ Local temp tables are stored as If we can identify that the name is that of a local temp table then adjust the identifier and query appropriately. + +Will warn user if \code{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. } \details{ True if: @@ -65,6 +104,9 @@ the table correctly unless name is adjusted ( allowed trailing wildcards to accomodate trailing underscores and postfix ). Therefore, in all cases query for \code{name___\%}. + +In both cases a simple CREATE TABLE statement is used / the table identifier is +is the differentiator ( viz-a-viz creating a non-temp table ). } \section{Failure modes}{ diff --git a/tests/testthat/_snaps/SQLServer.md b/tests/testthat/_snaps/SQLServer.md new file mode 100644 index 00000000..5a232d17 --- /dev/null +++ b/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 + diff --git a/tests/testthat/test-SQLServer.R b/tests/testthat/test-SQLServer.R index 2d78174f..71092c71 100644 --- a/tests/testthat/test-SQLServer.R +++ b/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 ) ) + }) })