Bug using prefixed function names with dbplyr

[akgold]

When using functions in dplyr against databases, prefixing certain functions seems to cause those functions to evaluate variable names in the global context, not the context of the data frame.

This isn't quite reprex-able, because I am not including making the connection.

In my case, con is a connection to a Postgres database using RPostgres. I suspect that doesn't matter based on how the error is showing up.

df <- tbl(con, "my_table")
> df %>% mutate(status = paste(subject, "abc")) #this works
# Source:   lazy query [?? x 18]
# Database: postgres [db@db.rds.amazonaws.com:5432/postgres]
... #excluded for confidentiality
> df %>% mutate(status = base::paste(subject, "abc"))
Error in base::paste(subject, "abc") : object 'subject' not found

[hadley]

Reprex with actual function involved in original problem:

library(dbplyr)
library(dplyr, warn.conflicts = FALSE)

df <- lazy_frame(subject = "x", con = simulate_postgres())
df %>% mutate(status = stringr::str_detect(subject, "abc"))
#> <SQL>
#> SELECT *, `subject` ~ 'abc' AS `status`
#> FROM `df`

^{Created on 2022-10-17 with reprex v2.0.2}

Which does work

[mgirlich]

@hadley There is a test that basically tests that functions with the base namespace are evaluated locally:

test_that("namespace operators always evaluated locally", {
  lf <- lazy_frame(x = 1, y = 2)

  expect_equal(partial_eval(quote(base::sum(1, 2)), lf), 3)
  expect_equal(partial_eval(quote(base:::sum(1, 2)), lf), 3)
})

but there is also the test to strip namespaces

test_that("namespaced calls to dplyr functions are stripped", {
  lf <- lazy_frame(x = 1, y = 2)

  expect_equal(partial_eval(quote(dplyr::n()), lf), expr(n()))
  expect_equal(partial_eval(quote(base::round(x)), lf), expr(round(x)))
  # hack to avoid check complaining about not declared imports
  expect_equal(partial_eval(rlang::parse_expr("stringr::str_to_lower(x)"), lf), expr(str_to_lower(x)))
  expect_equal(partial_eval(rlang::parse_expr("lubridate::today()"), lf), expr(today()))
})

Changing this might be a breaking change... What do you think?

[hadley]

I'm guessing I just missed this in #197; I don't think the base namespace should be an exception to the general rule. It seems unlikely to break much code since I don't think many people will have prefixed with base::.

[mgirlich]

Closed by 2dfcfba