Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL Server: correctly enumerate out of catalog schemas #692

Merged
merged 12 commits into from
Dec 20, 2023
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
Loading