Skip to content

Commit

Permalink
Generate NULL when collapsing empty vectors in SQL
Browse files Browse the repository at this point in the history
Fixes #318. Reverts #292.
  • Loading branch information
hadley committed Feb 29, 2024
1 parent f8aa8e8 commit 51f889c
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 15 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,4 @@ Config/Needs/website: bench, forcats, ggbeeswarm, ggplot2, R.utils,
Config/testthat/edition: 3
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3.9000
RoxygenNote: 7.3.1
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# glue (development version)

* `glue_sql("{var*}")` once again generates `NULL` if var is empty.
This reverts #292. (#318).

* The `.envir` argument to `glue()` and `glue_data()` really must be an
environment now, as documented. Previously a list-ish object worked in some
cases (by accident, not really by design). When you need to lookup values in
Expand Down
14 changes: 6 additions & 8 deletions R/sql.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@
#' values in a query, however not every database driver supports them.
#'
#' If you place a `*` at the end of a glue expression the values will be
#' collapsed with commas. This is useful for the [SQL IN Operator](https://www.w3schools.com/sql/sql_in.asp)
#' for instance.
#' collapsed with commas, or if there are no values, produce `NULL`.
#' This is useful for (e.g.) the
#' [SQL IN Operator](https://www.w3schools.com/sql/sql_in.asp).
#' @inheritParams glue
#' @seealso [glue_sql_collapse()] to collapse [DBI::SQL()] objects.
#' @param .con \[`DBIConnection`]: A DBI connection object obtained from
Expand Down Expand Up @@ -223,13 +224,10 @@ sql_quote_transformer <- function(connection, .na) {
}
} else {
res <- identity_transformer(text, envir)
if (length(res) == 0L) {
if (should_collapse) {
return("")
} else {
return(NULL)
}
if (length(res) == 0L && should_collapse) {
return(DBI::SQL("NULL"))
}

if (inherits(res, "SQL")) {
if (should_collapse) {
res <- glue_collapse(res, ", ")
Expand Down
5 changes: 3 additions & 2 deletions man/glue_sql.Rd

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

8 changes: 4 additions & 4 deletions tests/testthat/test-sql.R
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ describe("glue_sql", {
expect_identical(glue_sql("{var*}", .con = con, var = "a"), DBI::SQL("'a'"))
expect_identical(glue_sql("{var*}", .con = con, var = letters[1:5]), DBI::SQL("'a', 'b', 'c', 'd', 'e'"))
})
it('collapses empty values to empty string', {
expect_identical(glue_sql("{var*}", .con = con, var = character()), DBI::SQL(""))
expect_identical(glue_sql("{var*}", .con = con, var = DBI::SQL(character())), DBI::SQL(""))
it('collapses empty values to NULLempty ', {
expect_identical(glue_sql("{var*}", .con = con, var = character()), DBI::SQL("NULL"))
expect_identical(glue_sql("{var*}", .con = con, var = DBI::SQL(character())), DBI::SQL("NULL"))
})
it("mimics glue() when not collapsing", {
expect_equal(glue_sql("{var}", var = NULL), DBI::SQL(glue("{var}", var = NULL)))
expect_equal(glue_sql("{var}", .con = con, var = NULL), DBI::SQL(glue("{var}", var = NULL)))
})

it("should return an SQL NULL by default for missing values", {
Expand Down

0 comments on commit 51f889c

Please sign in to comment.