From d3fc05d4e48e673c879f3db8fd74b9eccc0f31b5 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 20 Feb 2024 08:23:33 -0600 Subject: [PATCH] Restore previous NULL behaviour in operators Fixes #1436 --- NEWS.md | 3 ++ R/translate-sql-helpers.R | 33 ++++++++++++--------- tests/testthat/test-translate-sql-helpers.R | 10 +++++-- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/NEWS.md b/NEWS.md index e73b501e4..be9211312 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # dbplyr (development version) +* You can once again use `NULL` on the LHS of an infix operator in order + to generate SQL with unusual syntax (#1345). + * Oracle once again translates `head()` to `FETCH FIRST`. This does require Oracle 12c or newer, but it actually works, compared to the approach using `ROWNUM` from #1292 (#1436). diff --git a/R/translate-sql-helpers.R b/R/translate-sql-helpers.R index baed5ad68..a26e7c4f7 100644 --- a/R/translate-sql-helpers.R +++ b/R/translate-sql-helpers.R @@ -153,29 +153,34 @@ sql_infix <- function(f, pad = TRUE) { # This is fixed with `escape_infix_expr()` # see https://github.com/tidyverse/dbplyr/issues/634 check_string(f) + check_bool(pad) f <- sql(f) - if (pad) { - function(x, y) { - x <- escape_infix_expr(enexpr(x), x) - y <- escape_infix_expr(enexpr(y), y) - - glue_sql2(sql_current_con(), "{.val x} {f} {.val y}") - } - } else { - function(x, y) { - x <- escape_infix_expr(enexpr(x), x) - y <- escape_infix_expr(enexpr(y), y) - - glue_sql2(sql_current_con(), "{.val x}{f}{.val y}") + function(x, y) { + x <- escape_infix_expr(enexpr(x), x) + y <- escape_infix_expr(enexpr(y), y) + + if (is.null(x)) { + if (pad) { + sql <- "{f} {.val y}" + } else { + sql <- "{f}{.val y}" + } + } else { + if (pad) { + sql <- "{.val x} {f} {.val y}" + } else { + sql <- "{.val x}{f}{.val y}" + } } + glue_sql2(sql_current_con(), sql) } } escape_infix_expr <- function(xq, x, escape_unary_minus = FALSE) { infix_calls <- c("+", "-", "*", "/", "%%", "^") is_infix <- is_call(xq, infix_calls, n = 2) - is_unary_minus <- escape_unary_minus && + is_unary_minus <- escape_unary_minus && is_call(xq, "-", n = 1) && !is_atomic(x, n = 1) if (is_infix || is_unary_minus) { diff --git a/tests/testthat/test-translate-sql-helpers.R b/tests/testthat/test-translate-sql-helpers.R index 34673286c..a6e8f1bae 100644 --- a/tests/testthat/test-translate-sql-helpers.R +++ b/tests/testthat/test-translate-sql-helpers.R @@ -66,11 +66,15 @@ test_that("unary minus works with expressions", { expect_equal(test_translate_sql(--x), sql("--`x`")) }) -test_that("pad = FALSE works", { +test_that("sql_infix generates expected output (#1345)", { local_con(simulate_dbi()) - subset <- sql_infix(".", pad = FALSE) + x <- ident_q("x") + y <- ident_q("y") - expect_equal(subset(ident("df"), ident("x")), sql("`df`.`x`")) + expect_equal(sql_infix("-", pad = FALSE)(x, y), sql("x-y")) + expect_equal(sql_infix("-", pad = FALSE)(NULL, y), sql("-y")) + expect_equal(sql_infix("-")(x, y), sql("x - y")) + expect_equal(sql_infix("-")(NULL, y), sql("- y")) }) test_that("sql_prefix checks arguments", {