Collapse empty vectors to empty string (#292)

* Collapse empty vectors to empty string Fixes #272 * Fix typo --------- Co-authored-by: Jennifer (Jenny) Bryan <jenny.f.bryan@gmail.com>
tidyverse · Mar 13, 2023 · 43e366e · 43e366e
1 parent 3a784a3
commit 43e366e
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 9 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,7 @@
 # glue (development version)
 
+* `glue_sql()` now collapses an empty vector to `""` not `"NULL"` (#272).
+
 * `glue_sql()` now uses `DBI::dbQuoteLiteral()` for all object types. This 
   should increase fidelity of escaping for different object types (#279).
 

diff --git a/R/sql.R b/R/sql.R
@@ -223,13 +223,17 @@ sql_quote_transformer <- function(connection, .na) {
       }
     } else {
       res <- eval(parse(text = text, keep.source = FALSE), envir)
+      if (length(res) == 0L) {
+        if (should_collapse) {
+          return("")
+        } else {
+          return(NULL)
+        }
+      }
       if (inherits(res, "SQL")) {
         if (should_collapse) {
           res <- glue_collapse(res, ", ")
         }
-        if (length(res) == 0L) {
-          res <- DBI::SQL("NULL")
-        }
         return(res)
       }
     }
@@ -243,9 +247,6 @@ sql_quote_transformer <- function(connection, .na) {
     if (should_collapse) {
       res <- glue_collapse(res, ", ")
     }
-    if (length(res) == 0L) {
-      res <- DBI::SQL("NULL")
-    }
     res
   }
 }
diff --git a/tests/testthat/test-sql.R b/tests/testthat/test-sql.R
@@ -49,10 +49,15 @@ 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 values should return NULL for length zero vector', {
-    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('collapses empty values to empty string', {
+    expect_identical(glue_sql("{var*}", .con = con, var = NULL), DBI::SQL(""))
+    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("mimics glue() when not collapsing", {
+    expect_equal(glue_sql("{var}", var = NULL), DBI::SQL(glue("{var}", var = NULL)))
+  })
+
   it("should return an SQL NULL by default for missing values", {
     var <- list(NA, NA_character_, NA_real_, NA_integer_)
     expect_identical(glue_sql("x = {var}", .con = con), rep(DBI::SQL("x = NULL"), 4))