Skip to content

Commit

Permalink
SQL Server: correctly enumerate out of catalog schemas (#692)
Browse files Browse the repository at this point in the history
* SQL Server: correctly enumerate out of catalog schemas
---------

Co-authored-by: Hadley Wickham <hadley@posit.co>
Co-authored-by: Simon P. Couch <simonpatrickcouch@gmail.com>
  • Loading branch information
3 people authored Dec 20, 2023
1 parent 6cdaaaa commit 99cde21
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 15 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# odbc (development version)

* SQL Server: correctly enumerate schemas across databases in connections pane
(@detule, #527).

* Added a function `odbcListConfig()` to help locate configuration files on
macOS and Linux (@simonpcouch, #565).

Expand Down
2 changes: 1 addition & 1 deletion R/driver-spark.R
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ setClass("Spark SQL", contains = "OdbcConnection")
#' This implementation will respect the `catalog_name` arrgument.
#' @rdname odbcConnectionSchemas
#' @usage NULL
setMethod("odbcConnectionSchemas", c("Spark SQL", "character"),
setMethod("odbcConnectionSchemas", "Spark SQL",
function(conn, catalog_name) {
res <- dbGetQuery(conn, paste0("SHOW SCHEMAS IN ", catalog_name))
if (nrow(res)) {
Expand Down
26 changes: 26 additions & 0 deletions R/driver-sql-server.R
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,32 @@ setMethod("dbExistsTable", c("Microsoft SQL Server", "SQL"),
}
)

#' @description
#' ## `odbcConnectionSchemas`
#' Call catalog-specific `sp_tables` to make sure we list the schemas in the
#' appropriate database/catalog.
#' @rdname SQLServer
#' @usage NULL
setMethod(
"odbcConnectionSchemas", "Microsoft SQL Server",
function(conn, catalog_name = NULL) {

if (is.null(catalog_name) || !nchar(catalog_name)) {
return(callNextMethod())
}
sproc <- paste(
dbQuoteIdentifier(conn, catalog_name), "dbo.sp_tables", sep = ".")

res <- dbGetQuery(conn, paste0(
"EXEC ", sproc, " ",
"@table_name = '', ",
"@table_owner = '%', ",
"@table_qualifier = ''"
))
res$TABLE_OWNER
}
)

#' @rdname SQLServer
#' @description
#' ## `sqlCreateTable()`
Expand Down
9 changes: 1 addition & 8 deletions R/odbc-connection.R
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ setMethod("odbcConnectionCatalogs", "OdbcConnection",
setGeneric(
"odbcConnectionSchemas",
valueClass = "character",
function(conn, catalog_name) {
function(conn, catalog_name = NULL) {
standardGeneric("odbcConnectionSchemas")
}
)
Expand All @@ -406,13 +406,6 @@ setMethod("odbcConnectionSchemas", "OdbcConnection",
}
)

#' @rdname odbcConnectionSchemas
setMethod("odbcConnectionSchemas", c("OdbcConnection", "character"),
function(conn, catalog_name) {
connection_sql_schemas(conn@ptr)
}
)

#' odbcConnectionTableTypes
#'
#' This function returns a listing of table
Expand Down
5 changes: 5 additions & 0 deletions man/SQLServer.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 3 additions & 6 deletions man/odbcConnectionSchemas.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions tests/testthat/test-driver-sql-server.R
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,18 @@ test_that("SQLServer", {
dbWriteTable(conn = con, name = table_id, value = ir, overwrite = TRUE)
res <- dbReadTable(con, table_id)
expect_equal(res, ir)

# Test: We can enumerate schemas out of catalog ( #527 )
# Part 1: Make sure we can see the schema we created in the
# current catalog.
res <- odbcConnectionSchemas(con)
expect_true("testSchema" %in% res)
# Part 2: Make sure we don't see that schema in the tempdb
# listing ( out of catalog schema listing )
res <- odbcConnectionSchemas(con, catalog_name = "tempdb")
# Should, at least, have INFORMATION_SCHEMA and sys
expect_true(length(res) > 1)
expect_false("testSchema" %in% res)
})

local({
Expand Down

0 comments on commit 99cde21

Please sign in to comment.