Skip to content

Commit

Permalink
Soften rules for interpolating values
Browse files Browse the repository at this point in the history
Partially reverts #1368. See bcgov/bcdata#338 for details. If we do this again in the future, I think we have to do argument checking in individual translations. That's a lot of work but allows a whole suite of potentially useful translations for more complex R objects.
  • Loading branch information
hadley committed Feb 22, 2024
1 parent edde2a6 commit 196900a
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 24 deletions.
3 changes: 0 additions & 3 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@

* The databricks backend now supports creating non-temporary tables too (#1418).

* Clearer error if you attempt to embed non-atomic vectors inside of a generated
query (#1368).

* `x$name` never attempts to evaluate `name` (#1368).

* `rows_patch(in_place = FALSE)` now works when more than one column should be
Expand Down
3 changes: 2 additions & 1 deletion R/tidyeval.R
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,11 @@ partial_eval_sym <- function(sym, data, env) {
error_embed("shiny inputs", paste0(name, "$x"))
}


if (is_sql_literal(val)) {
unname(val)
} else {
error_embed(obj_type_friendly(val), name)
val
}
} else {
cli::cli_abort(
Expand Down
7 changes: 2 additions & 5 deletions tests/testthat/_snaps/backend-.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@
Code
lazy_frame(x = 1) %>% filter(x == y$id)
Condition
Error in `filter()`:
i In argument: `x == y$id`
Caused by error:
! Cannot translate a list to SQL.
i Do you want to force evaluation in R with (e.g.) `!!y` or `local(y)`?
Error in `<list: id = 1>$id`:
! `$` can only subset database columns, not inlined values.

# useful error if $ used with inlined value

Expand Down
12 changes: 0 additions & 12 deletions tests/testthat/_snaps/tidyeval.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,4 @@
Error:
! Cannot translate a data.frame to SQL.
i Do you want to force evaluation in R with (e.g.) `!!df$x` or `local(df$x)`?
Code
capture_dot(lf, l)
Condition
Error:
! Cannot translate an empty list to SQL.
i Do you want to force evaluation in R with (e.g.) `!!l` or `local(l)`?
Code
capture_dot(lf, mean)
Condition
Error:
! Cannot translate a function to SQL.
i Do you want to force evaluation in R with (e.g.) `!!mean` or `local(mean)`?

3 changes: 0 additions & 3 deletions tests/testthat/test-tidyeval.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,12 @@ test_that("other objects get informative error", {

input <- structure(list(), class = "reactivevalues")
x <- structure(function() "y", class = "reactive")
l <- list()
df <- data.frame(x = 1)

expect_snapshot({
capture_dot(lf, input)
capture_dot(lf, x())
capture_dot(lf, df)
capture_dot(lf, l)
capture_dot(lf, mean)
}, error = TRUE)
})

Expand Down

0 comments on commit 196900a

Please sign in to comment.