From 43e366e91d3b713c1a82fb6e3f69ade0cb121d73 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 13 Mar 2023 15:01:31 -0500 Subject: [PATCH] Collapse empty vectors to empty string (#292) * Collapse empty vectors to empty string Fixes #272 * Fix typo --------- Co-authored-by: Jennifer (Jenny) Bryan --- NEWS.md | 2 ++ R/sql.R | 13 +++++++------ tests/testthat/test-sql.R | 11 ++++++++--- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index db41dd1..4c49e29 100644 --- 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 index afbbd52..6290ec7 100644 --- 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 index c64fbfd..0a516cf 100644 --- 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))