Translate evaluated expression

NEWS.md

@@ -1,5 +1,12 @@
 # dbplyr (development version)
 
+* Partially evaluated expressions with infix operations are now correctly
+  translated. For example `translate_sql(!!expr(2 - 1) * x)` now works
+  (@mgirlich, #634).
+
+* Expressions with a unary plus do not produce an error anymore. For example
+  `lazy_frame(x = 1) %>% filter(x == +1)` now works (@mgirlich, #674).
+
 * Joins with `na_matches = "na"` now work for DuckDB (@mgirlich, #704).
 
 * `nesting()` now supports the `.name_repair` argument (@mgirlich, #654).
@@ -11,7 +18,6 @@
 
 * `if_else()` now supports the `missing` argument (@mgirlich, #641).
 
-
 # dbplyr 2.1.1
 
 * New support for Snowflake (@edgararuiz)

R/backend-.R

@@ -22,20 +22,36 @@ sql_translation.DBIConnection <- function(con) {
 #' @rdname sql_variant
 #' @format NULL
 base_scalar <- sql_translator(
-  `+`    = sql_infix("+"),
+  `+`    = function(x, y = NULL) {
+    x <- escape_infix_expr(enexpr(x), x, escape_unary_minus = TRUE)
+    if (is.null(y)) {
+      if (is.numeric(x)) {
+        x
+      } else {
+        sql_expr(!!x)
+      }
+    } else {
+      y <- escape_infix_expr(enexpr(y), y)
+
+      sql_expr(!!x + !!y)
+    }
+  },
   `*`    = sql_infix("*"),
   `/`    = sql_infix("/"),
   `%/%`  = sql_not_supported("%/%"),
   `%%`   = sql_infix("%"),
   `^`    = sql_prefix("POWER", 2),
   `-`    = function(x, y = NULL) {
+    x <- escape_infix_expr(enexpr(x), x, escape_unary_minus = TRUE)
     if (is.null(y)) {
       if (is.numeric(x)) {
         -x
       } else {
         sql_expr(-!!x)
       }
     } else {
+      y <- escape_infix_expr(enexpr(y), y)
+
       sql_expr(!!x - !!y)
     }
   },

R/translate-sql-helpers.R

@@ -132,19 +132,47 @@ copy_env <- function(from, to = NULL, parent = parent.env(from)) {
 #' @param pad If `TRUE`, the default, pad the infix operator with spaces.
 #' @export
 sql_infix <- function(f, pad = TRUE) {
+  # Unquoting involving infix operators easily create abstract syntax trees
+  # without parantheses where they are needed for printing and translation.
+  # For example `expr(!!expr(2 - 1) * x))`
+  #
+  # See https://adv-r.hadley.nz/quasiquotation.html#non-standard-ast
+  # for more information.
+  #
+  # This is fixed with `escape_infix_expr()`
+  # see https://github.com/tidyverse/dbplyr/issues/634
   assert_that(is_string(f))
 
   if (pad) {
     function(x, y) {
+      x <- escape_infix_expr(enexpr(x), x)
+      y <- escape_infix_expr(enexpr(y), y)
+
       build_sql(x, " ", sql(f), " ", y)
     }
   } else {
     function(x, y) {
+      x <- escape_infix_expr(enexpr(x), x)
+      y <- escape_infix_expr(enexpr(y), y)
+
       build_sql(x, sql(f), y)
     }
   }
 }
 
+escape_infix_expr <- function(xq, x, escape_unary_minus = FALSE) {
+  infix_calls <- c("+", "-", "*", "/", "%%", "^")
+  if (is_call(xq, infix_calls, n = 2)) {
+    return(build_sql("(", x, ")"))
+  }
+
+  if (escape_unary_minus && is_call(xq, "-", n = 1)) {
+    return(build_sql("(", x, ")"))
+  }
+
+  x
+}
+
 #' @rdname sql_variant
 #' @export
 sql_prefix <- function(f, n = NULL) {

tests/testthat/test-backend-.R

@@ -13,6 +13,17 @@ test_that("small numbers aren't converted to 0", {
   expect_equal(translate_sql(1e-9), sql("1e-09"))
 })
 
+test_that("unary plus works with numbers", {
+  expect_equal(translate_sql(+10L), sql("10"))
+  expect_equal(translate_sql(x == +10), sql('`x` = 10.0'))
+  expect_equal(translate_sql(x %in% c(+1L, 0L)), sql('`x` IN (1, 0)'))
+})
+
+test_that("unary plus works for non-numeric expressions", {
+  expect_equal(translate_sql(+(1L + 2L)), sql("(1 + 2)"))
+  expect_equal(translate_sql(mean(x, na.rm = TRUE), window = FALSE), sql('AVG(`x`)'))
+})
+
 test_that("unary minus flips sign of number", {
   expect_equal(translate_sql(-10L), sql("-10"))
   expect_equal(translate_sql(x == -10), sql('`x` = -10.0'))

tests/testthat/test-translate-sql-helpers.R

@@ -42,3 +42,16 @@ test_that("win_rank() is accepted by the sql_translator", {
     )
   )
 })
+
+test_that("can translate infix expression without parantheses", {
+  expect_equal(translate_sql(!!expr(2 - 1) * x), sql("(2.0 - 1.0) * `x`"))
+  expect_equal(translate_sql(!!expr(2 / 1) * x), sql("(2.0 / 1.0) * `x`"))
+
+  expect_equal(translate_sql(!!expr(2 * 1) - x), sql("(2.0 * 1.0) - `x`"))
+})
+
+test_that("unary minus works with expressions", {
+  expect_equal(translate_sql(-!!rlang::expr(x+2)), sql("-(`x` + 2.0)"))
+
+  expect_equal(translate_sql(--x), sql("-(-`x`)"))
+})