From ee215d58712a058be16b77bef50437b476637fc3 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 7 Nov 2023 07:44:32 -0600 Subject: [PATCH 1/5] Remove requirement to name ID components --- R/00-Id.R | 30 ++++++++++++++---------------- man/Id.Rd | 21 +++++++++------------ man/dbClearResult.Rd | 9 +++------ tests/testthat/_snaps/00-Id.md | 15 +++++++++++++++ tests/testthat/_snaps/00-Id.new.md | 15 +++++++++++++++ tests/testthat/test-00-Id.R | 7 +++++++ 6 files changed, 63 insertions(+), 34 deletions(-) create mode 100644 tests/testthat/_snaps/00-Id.md create mode 100644 tests/testthat/_snaps/00-Id.new.md create mode 100644 tests/testthat/test-00-Id.R diff --git a/R/00-Id.R b/R/00-Id.R index c353357e1..fbbe51479 100644 --- a/R/00-Id.R +++ b/R/00-Id.R @@ -6,10 +6,10 @@ setClass("Id", slots = list(name = "character")) #' Refer to a table nested in a hierarchy (e.g. within a schema) #' #' @description -#' Objects of class `Id` have a single slot `name`, which is a named -#' character vector. +#' Objects of class `Id` have a single slot `name`, which is a vector. #' The [dbQuoteIdentifier()] method converts `Id` objects to strings. #' Support for `Id` objects depends on the database backend. +#' #' They can be used in the following methods as `name` or `table` argument: #' #' - [dbCreateTable()] @@ -21,35 +21,33 @@ setClass("Id", slots = list(name = "character")) #' #' Objects of this class are also returned from [dbListObjects()]. #' -#' @param ... Components of the hierarchy, e.g. `schema`, `table`, -#' or `cluster`, `catalog`, `schema`, `table`, -#' depending on the database backend. -#' For more on these concepts, see -#' \url{https://stackoverflow.com/questions/7022755/} +#' @param ... Components of the hierarchy, e.g. `schema`, `table`, or `cluster`, +#' `catalog`, `schema`, `table`, depending on the database backend. For more +#' on these concepts, see #' @export #' @examples #' # Identifies a table in a specific schema: +#' Id("dbo", "Customer") +#' # You can name the components if you want, but it's not needed #' Id(schema = "dbo", table = "Customer") #' -#' # Identifies a table in a specific cluster, catalog, and schema: -#' Id(cluster = "mycluster", catalog = "mycatalog", schema = "myschema", table = "mytable") -#' #' # Create a SQL expression for an identifier: -#' dbQuoteIdentifier(ANSI(), Id(schema = "nycflights13", table = "flights")) +#' dbQuoteIdentifier(ANSI(), Id("nycflights13", "flights")) #' #' # Write a table in a specific schema: #' \dontrun{ -#' dbWriteTable(con, Id(schema = "myschema", table = "mytable"), data.frame(a = 1)) +#' dbWriteTable(con, Id("myschema", "mytable"), data.frame(a = 1)) #' } Id <- function(...) { components <- c(...) - if (is.null(names(components)) || any(names(components) == "")) { - stop("All arguments to Id() must be named.", call. = FALSE) + if (!is.character(components)) { + stop("All elements of `...` must be strings.", call. = FALSE) } - new("Id", name = components) + + new("Id", name = c(...)) } #' @export toString.Id <- function(x, ...) { - paste0(" ", paste0(names(x@name), " = ", x@name, collapse = ", ")) + paste0(" ", paste0('"', x@name, '"', collapse = ".")) } diff --git a/man/Id.Rd b/man/Id.Rd index 61de52220..702766e5b 100644 --- a/man/Id.Rd +++ b/man/Id.Rd @@ -9,17 +9,15 @@ Id(...) } \arguments{ -\item{...}{Components of the hierarchy, e.g. \code{schema}, \code{table}, -or \code{cluster}, \code{catalog}, \code{schema}, \code{table}, -depending on the database backend. -For more on these concepts, see -\url{https://stackoverflow.com/questions/7022755/}} +\item{...}{Components of the hierarchy, e.g. \code{schema}, \code{table}, or \code{cluster}, +\code{catalog}, \code{schema}, \code{table}, depending on the database backend. For more +on these concepts, see \url{https://stackoverflow.com/questions/7022755/}} } \description{ -Objects of class \code{Id} have a single slot \code{name}, which is a named -character vector. +Objects of class \code{Id} have a single slot \code{name}, which is a vector. The \code{\link[=dbQuoteIdentifier]{dbQuoteIdentifier()}} method converts \code{Id} objects to strings. Support for \code{Id} objects depends on the database backend. + They can be used in the following methods as \code{name} or \code{table} argument: \itemize{ \item \code{\link[=dbCreateTable]{dbCreateTable()}} @@ -34,16 +32,15 @@ Objects of this class are also returned from \code{\link[=dbListObjects]{dbListO } \examples{ # Identifies a table in a specific schema: +Id("dbo", "Customer") +# You can name the components if you want, but it's not needed Id(schema = "dbo", table = "Customer") -# Identifies a table in a specific cluster, catalog, and schema: -Id(cluster = "mycluster", catalog = "mycatalog", schema = "myschema", table = "mytable") - # Create a SQL expression for an identifier: -dbQuoteIdentifier(ANSI(), Id(schema = "nycflights13", table = "flights")) +dbQuoteIdentifier(ANSI(), Id("nycflights13", "flights")) # Write a table in a specific schema: \dontrun{ -dbWriteTable(con, Id(schema = "myschema", table = "mytable"), data.frame(a = 1)) +dbWriteTable(con, Id("myschema", "mytable"), data.frame(a = 1)) } } diff --git a/man/dbClearResult.Rd b/man/dbClearResult.Rd index 75798fb7f..8d49e4e9e 100644 --- a/man/dbClearResult.Rd +++ b/man/dbClearResult.Rd @@ -13,9 +13,8 @@ dbClearResult(res, ...) } \value{ \code{dbClearResult()} returns \code{TRUE}, invisibly, for result sets obtained from -\code{dbSendQuery()}, -\code{dbSendStatement()}, -or \code{dbSendQueryArrow()}, +both \code{dbSendQuery()} +and \code{dbSendStatement()}. } \description{ Frees all resources (local and remote) associated with a result set. @@ -93,9 +92,7 @@ to ensure that this step is always executed. An attempt to close an already closed result set issues a warning -for \code{dbSendQuery()}, -\code{dbSendStatement()}, -and \code{dbSendQueryArrow()}, +in both cases. } diff --git a/tests/testthat/_snaps/00-Id.md b/tests/testthat/_snaps/00-Id.md new file mode 100644 index 000000000..5379cd646 --- /dev/null +++ b/tests/testthat/_snaps/00-Id.md @@ -0,0 +1,15 @@ +# Id() requires a character vector + + Code + Id(1) + Condition + Error: + ! All elements of `...` must be strings. + +# has a decent print method + + Code + Id("a", "b") + Output + a.b + diff --git a/tests/testthat/_snaps/00-Id.new.md b/tests/testthat/_snaps/00-Id.new.md new file mode 100644 index 000000000..694ddfaa3 --- /dev/null +++ b/tests/testthat/_snaps/00-Id.new.md @@ -0,0 +1,15 @@ +# Id() requires a character vector + + Code + Id(1) + Condition + Error: + ! All elements of `...` must be strings. + +# has a decent print method + + Code + Id("a", "b") + Output + "a"."b" + diff --git a/tests/testthat/test-00-Id.R b/tests/testthat/test-00-Id.R new file mode 100644 index 000000000..e9a7331e0 --- /dev/null +++ b/tests/testthat/test-00-Id.R @@ -0,0 +1,7 @@ +test_that("Id() requires a character vector", { + expect_snapshot(Id(1), error = TRUE) +}) + +test_that("has a decent print method", { + expect_snapshot(Id("a", "b")) +}) From 9ca18980a4bf713db53eb3072fadaae8424b4d14 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 7 Nov 2023 07:48:34 -0600 Subject: [PATCH 2/5] Move dbQuoteIdentifer implementation --- R/00-Id.R | 7 ++++++- R/dbQuoteIdentifier_DBIConnection.R | 5 +---- tests/testthat/_snaps/00-Id.md | 2 +- tests/testthat/_snaps/00-Id.new.md | 15 --------------- tests/testthat/test-00-Id.R | 7 +++++++ 5 files changed, 15 insertions(+), 21 deletions(-) delete mode 100644 tests/testthat/_snaps/00-Id.new.md diff --git a/R/00-Id.R b/R/00-Id.R index fbbe51479..e20dfd4be 100644 --- a/R/00-Id.R +++ b/R/00-Id.R @@ -49,5 +49,10 @@ Id <- function(...) { #' @export toString.Id <- function(x, ...) { - paste0(" ", paste0('"', x@name, '"', collapse = ".")) + paste0(" ", dbQuoteIdentifier(ANSI(), x)) +} + + +dbQuoteIdentifier_DBIConnection_Id <- function(conn, x, ...) { + SQL(paste0(dbQuoteIdentifier(conn, x@name), collapse = ".")) } diff --git a/R/dbQuoteIdentifier_DBIConnection.R b/R/dbQuoteIdentifier_DBIConnection.R index 2fe31550f..05099f65c 100644 --- a/R/dbQuoteIdentifier_DBIConnection.R +++ b/R/dbQuoteIdentifier_DBIConnection.R @@ -3,9 +3,6 @@ dbQuoteIdentifier_DBIConnection <- function(conn, x, ...) { # Don't support lists, auto-vectorization violates type stability if (is(x, "SQL")) return(x) - if (is(x, "Id")) { - return(SQL(paste0(dbQuoteIdentifier(conn, x@name), collapse = "."))) - } if (!is.character(x)) stop("x must be character or SQL", call. = FALSE) if (any(is.na(x))) { @@ -37,4 +34,4 @@ setMethod("dbQuoteIdentifier", signature("DBIConnection", "SQL"), dbQuoteIdentif #' @rdname hidden_aliases #' @export -setMethod("dbQuoteIdentifier", signature("DBIConnection", "Id"), dbQuoteIdentifier_DBIConnection) +setMethod("dbQuoteIdentifier", signature("DBIConnection", "Id"), dbQuoteIdentifier_DBIConnection_Id) diff --git a/tests/testthat/_snaps/00-Id.md b/tests/testthat/_snaps/00-Id.md index 5379cd646..694ddfaa3 100644 --- a/tests/testthat/_snaps/00-Id.md +++ b/tests/testthat/_snaps/00-Id.md @@ -11,5 +11,5 @@ Code Id("a", "b") Output - a.b + "a"."b" diff --git a/tests/testthat/_snaps/00-Id.new.md b/tests/testthat/_snaps/00-Id.new.md deleted file mode 100644 index 694ddfaa3..000000000 --- a/tests/testthat/_snaps/00-Id.new.md +++ /dev/null @@ -1,15 +0,0 @@ -# Id() requires a character vector - - Code - Id(1) - Condition - Error: - ! All elements of `...` must be strings. - -# has a decent print method - - Code - Id("a", "b") - Output - "a"."b" - diff --git a/tests/testthat/test-00-Id.R b/tests/testthat/test-00-Id.R index e9a7331e0..1c408f360 100644 --- a/tests/testthat/test-00-Id.R +++ b/tests/testthat/test-00-Id.R @@ -5,3 +5,10 @@ test_that("Id() requires a character vector", { test_that("has a decent print method", { expect_snapshot(Id("a", "b")) }) + +test_that("each element is quoted individually", { + expect_equal( + DBI::dbQuoteIdentifier(ANSI(), Id("a", "b.c")), + SQL('"a"."b.c"') + ) +}) From fda206e2980addffe1310a58229a6ab1fc68ac7d Mon Sep 17 00:00:00 2001 From: hadley Date: Tue, 7 Nov 2023 13:51:00 +0000 Subject: [PATCH 3/5] Auto-update from GitHub Actions Run: https://github.com/r-dbi/DBI/actions/runs/6785413415 --- man/dbClearResult.Rd | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/man/dbClearResult.Rd b/man/dbClearResult.Rd index 8d49e4e9e..75798fb7f 100644 --- a/man/dbClearResult.Rd +++ b/man/dbClearResult.Rd @@ -13,8 +13,9 @@ dbClearResult(res, ...) } \value{ \code{dbClearResult()} returns \code{TRUE}, invisibly, for result sets obtained from -both \code{dbSendQuery()} -and \code{dbSendStatement()}. +\code{dbSendQuery()}, +\code{dbSendStatement()}, +or \code{dbSendQueryArrow()}, } \description{ Frees all resources (local and remote) associated with a result set. @@ -92,7 +93,9 @@ to ensure that this step is always executed. An attempt to close an already closed result set issues a warning -in both cases. +for \code{dbSendQuery()}, +\code{dbSendStatement()}, +and \code{dbSendQueryArrow()}, } From bb53d5c42a7ce68e3dc04c33dff1dc641a27e4a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Wed, 8 Nov 2023 13:48:37 +0100 Subject: [PATCH 4/5] Clarify type --- R/00-Id.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/00-Id.R b/R/00-Id.R index e20dfd4be..b196513ce 100644 --- a/R/00-Id.R +++ b/R/00-Id.R @@ -6,7 +6,7 @@ setClass("Id", slots = list(name = "character")) #' Refer to a table nested in a hierarchy (e.g. within a schema) #' #' @description -#' Objects of class `Id` have a single slot `name`, which is a vector. +#' Objects of class `Id` have a single slot `name`, which is a character vector. #' The [dbQuoteIdentifier()] method converts `Id` objects to strings. #' Support for `Id` objects depends on the database backend. #' From 9dd810e60e7ddbae93d9ae938f208fdad8a0492e Mon Sep 17 00:00:00 2001 From: krlmlr Date: Wed, 8 Nov 2023 12:50:55 +0000 Subject: [PATCH 5/5] Auto-update from GitHub Actions Run: https://github.com/r-dbi/DBI/actions/runs/6798438518 --- man/Id.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/Id.Rd b/man/Id.Rd index 702766e5b..6549d4825 100644 --- a/man/Id.Rd +++ b/man/Id.Rd @@ -14,7 +14,7 @@ Id(...) on these concepts, see \url{https://stackoverflow.com/questions/7022755/}} } \description{ -Objects of class \code{Id} have a single slot \code{name}, which is a vector. +Objects of class \code{Id} have a single slot \code{name}, which is a character vector. The \code{\link[=dbQuoteIdentifier]{dbQuoteIdentifier()}} method converts \code{Id} objects to strings. Support for \code{Id} objects depends on the database backend.