From 51f889cea9092d59c83b7d3986a70f93f07f330c Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Thu, 29 Feb 2024 08:17:00 -0600
Subject: [PATCH] Generate NULL when collapsing empty vectors in SQL

Fixes #318. Reverts #292.
---
 DESCRIPTION               |  2 +-
 NEWS.md                   |  3 +++
 R/sql.R                   | 14 ++++++--------
 man/glue_sql.Rd           |  5 +++--
 tests/testthat/test-sql.R |  8 ++++----
 5 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index 33bf294..eca517e 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -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
diff --git a/NEWS.md b/NEWS.md
index ba3faf6..f83c8fd 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -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
diff --git a/R/sql.R b/R/sql.R
index ac03cde..81cc839 100644
--- a/R/sql.R
+++ b/R/sql.R
@@ -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
@@ -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, ", ")
diff --git a/man/glue_sql.Rd b/man/glue_sql.Rd
index a2d83a1..361ec90 100644
--- a/man/glue_sql.Rd
+++ b/man/glue_sql.Rd
@@ -104,8 +104,9 @@ are generally the safest and most efficient way to pass user defined
 values in a query, however not every database driver supports them.
 
 If you place a \code{*} at the end of a glue expression the values will be
-collapsed with commas. This is useful for the \href{https://www.w3schools.com/sql/sql_in.asp}{SQL IN Operator}
-for instance.
+collapsed with commas, or if there are no values, produce \code{NULL}.
+This is useful for (e.g.) the
+\href{https://www.w3schools.com/sql/sql_in.asp}{SQL IN Operator}.
 }
 \examples{
 \dontshow{if (requireNamespace("DBI", quietly = TRUE) && requireNamespace("RSQLite", quietly = TRUE)) (if (getRversion() >= "3.4") withAutoprint else force)(\{ # examplesIf}
diff --git a/tests/testthat/test-sql.R b/tests/testthat/test-sql.R
index cd1ae58..7f6cb65 100644
--- a/tests/testthat/test-sql.R
+++ b/tests/testthat/test-sql.R
@@ -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", {