From 3a784a3d3c0ec3b234cb5318a545d19bd53b3589 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Mon, 13 Mar 2023 14:58:22 -0500
Subject: [PATCH] Always use DBI::dbQuoteLiteral() (#293)

Fixes #279

Co-authored-by: Jennifer (Jenny) Bryan <jenny.f.bryan@gmail.com>
---
 DESCRIPTION               |  2 +-
 NEWS.md                   |  3 +++
 R/sql.R                   | 20 ++++++--------------
 tests/testthat/test-sql.R | 19 ++++++++++---------
 4 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index 91570c97..de62f5e9 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -23,7 +23,7 @@ Imports:
     methods
 Suggests: 
     crayon,
-    DBI,
+    DBI (>= 1.1.3),
     dplyr,
     knitr,
     magrittr,
diff --git a/NEWS.md b/NEWS.md
index d2c7801b..db41dd11 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,8 @@
 # glue (development version)
 
+* `glue_sql()` now uses `DBI::dbQuoteLiteral()` for all object types. This 
+  should increase fidelity of escaping for different object types (#279).
+
 * The "Speed of glue" vignette has been converted to an article, which allows several package to be removed from `Suggests` (and re-located to `Config/Needs/website`). The code got a light refresh, including a switch from microbenchmark to bench and more modern use of ggplot2.
 
 * Add `$(C_VISIBILITY)` to compiler flags to hide internal symbols from the dll (#284 @lionel-).
diff --git a/R/sql.R b/R/sql.R
index 3b300637..afbbd52c 100644
--- a/R/sql.R
+++ b/R/sql.R
@@ -232,22 +232,14 @@ sql_quote_transformer <- function(connection, .na) {
         }
         return(res)
       }
+    }
 
-      # convert objects to characters
-      is_object <- is.object(res)
-      if (is_object) {
-        res <- as.character(res)
-      }
-
-      is_na <- is.na(res)
-      if (any(is_na)) {
-        res[is_na] <- rep(list(.na), sum(is_na))
-      }
-
-      is_char <- vapply(res, function(x) !is.na(x) && is.character(x), logical(1))
-      res[is_char] <- lapply(res[is_char], function(x) DBI::dbQuoteLiteral(conn = connection, x))
-      res[!is_char] <- lapply(res[!is_char], function(x) DBI::SQL(conn = connection, x))
+    if (is.list(res)) {
+      res <- unlist(lapply(res, DBI::dbQuoteLiteral, conn = connection))
+    } else {
+      res <- DBI::dbQuoteLiteral(connection, res)
     }
+
     if (should_collapse) {
       res <- glue_collapse(res, ", ")
     }
diff --git a/tests/testthat/test-sql.R b/tests/testthat/test-sql.R
index 1b16d02e..c64fbfd5 100644
--- a/tests/testthat/test-sql.R
+++ b/tests/testthat/test-sql.R
@@ -29,7 +29,10 @@ describe("glue_sql", {
       DBI::Id(schema = "foo", table = "bar", column = "baz"),
       DBI::Id(schema = "foo", table = "bar", column = "baz2")
     )
-    expect_identical(glue_sql("{`var`*}", .con = con), DBI::SQL("`foo`.`bar`.`baz`, `foo`.`bar`.`baz2`"))
+    expect_identical(
+      glue_sql("{`var`*}", .con = con),
+      DBI::SQL("`foo`.`bar`.`baz`, `foo`.`bar`.`baz2`")
+    )
   })
   it("Does not quote numbers", {
     var <- 1
@@ -55,16 +58,11 @@ describe("glue_sql", {
     expect_identical(glue_sql("x = {var}", .con = con), rep(DBI::SQL("x = NULL"), 4))
   })
 
-  it("should return NA for missing values and .na = NULL", {
-    var <- list(NA, NA_character_, NA_real_, NA_integer_)
-    expect_identical(glue_sql("x = {var}", .con = con, .na = NULL), rep(DBI::SQL(NA), 4))
-  })
-
   it("should preserve the type of the even with missing values (#130)", {
       expect_identical(glue_sql("x = {c(1L, NA)}", .con = con), DBI::SQL(c(paste0("x = ", c(1, "NULL")))))
       expect_identical(glue_sql("x = {c(1, NA)}", .con = con), DBI::SQL(c(paste0("x = ", c(1, "NULL")))))
       expect_identical(glue_sql("x = {c('1', NA)}", .con = con), DBI::SQL(c(paste0("x = ", c("'1'", "NULL")))))
-      expect_identical(glue_sql("x = {c(TRUE, NA)}", .con = con), DBI::SQL(c(paste0("x = ", c("TRUE", "NULL")))))
+      expect_identical(glue_sql("x = {c(TRUE, NA)}", .con = con), DBI::SQL(c(paste0("x = ", c("1", "NULL")))))
   })
 
   it("should return NA for missing values quote strings", {
@@ -79,14 +77,17 @@ describe("glue_sql", {
 
   it("should quote values from lists properly", {
     var <- list(1, 2, "three")
-    expect_identical(glue_sql("x = {var}", .con = con), DBI::SQL(c("x = 1", "x = 2", "x = 'three'")))
+    expect_identical(
+      glue_sql("x = {var}", .con = con),
+      DBI::SQL(c("x = 1", "x = 2", "x = 'three'"))
+    )
   })
 
   it("should handle NA when collapsing (#185)", {
     expect_identical(glue_sql("x IN ({c(NA, 'A')*})", .con = con), DBI::SQL(paste0("x IN (NULL, 'A')")))
     expect_identical(glue_sql("x IN ({c(NA, 1)*})", .con = con), DBI::SQL(paste0("x IN (NULL, 1)")))
     expect_identical(glue_sql("x IN ({c(NA, 1L)*})", .con = con), DBI::SQL(paste0("x IN (NULL, 1)")))
-    expect_identical(glue_sql("x IN ({c(NA, TRUE)*})", .con = con), DBI::SQL(paste0("x IN (NULL, TRUE)")))
+    expect_identical(glue_sql("x IN ({c(NA, TRUE)*})", .con = con), DBI::SQL(paste0("x IN (NULL, 1)")))
   })
 
   it("should handle DBI::SQL() elements correctly when collapsing (#191)", {