bcgov / bcdata

An R package for searching & retrieving data from the B.C. Data Catalogue
https://bcgov.github.io/bcdata
Apache License 2.0
81 stars 12 forks source link

cql_translate is not robust to filter statements containing local objects that have names of other objects #280

Closed ateucher closed 11 months ago

ateucher commented 3 years ago
library(bcdata)

x <- c("a", "b")
y <- data.frame(id = x, stringsAsFactors = FALSE)

bcdata:::cql_translate(foo == y$id[2])
#> <SQL> ("foo" = 'b')

library(dplyr)

bcdata:::cql_translate(foo == y$id[2])
#> Error in structure(list(id = c("a", "b")), class = "data.frame", row.names = c(NA, : invalid subscript type 'closure'

traceback()
#> 11: eval_bare(x, .env)
#> 10: .f(.x[[i]], ...)
#> 9: purrr::map_chr(enexprs(...), escape_expr, con = con)
#> 8: build_sql(x, " ", sql(f), " ", y)
#> 7: foo == structure(list(id = c("a", "b")), class = "data.frame", row.names = c(NA, 
#>                                                                                 -2L))$function (.variables, drop = FALSE) 
#>                                                                                 {
#>                                                                                   lifecycle::deprecate_stop("0.5.0", "id()", "vctrs::vec_group_id()")
#>                                                                                 }[2]
#> 6: eval_tidy(x, mask)
#> 5: escape(eval_tidy(x, mask), con = con)
#> 4: FUN(X[[i]], ...)
#> 3: lapply(dots, function(x) {
#>   if (is_null(get_expr(x))) {
#>     NULL
#>   }
#>   else if (is_atomic(get_expr(x))) {
#>     escape(get_expr(x), con = con)
#>   }
#>   else {
#>     mask <- sql_data_mask(x, variant, con = con, window = window)
#>     escape(eval_tidy(x, mask), con = con)
#>   }
#> })
#> 2: dbplyr::translate_sql_(dots, con = wfs_con, window = FALSE) at cql-translator.R#52
#> 1: bcdata:::cql_translate(foo == y$id[2])

This section is not successfully wrapping the y$id[[2]] in "local()", so it then finds dplyr::id() instead of evaluating y$id[[2]] to get the value ("b").

ateucher commented 11 months ago

This is now handled by explicitly wrapping locally evaluated expressions in local(), and supplying column names for remote columns in the .colnames argument. The local() has to be done by the user, the .colnames are handled internally. It was closed by #305.

x <- c("a", "b")
y <- data.frame(id = x, stringsAsFactors = FALSE)

bcdata:::cql_translate(foo == local(y$id[2]), .colnames = "foo")
#> <SQL> ("foo" = 'b')

library(dplyr)
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union
bcdata:::cql_translate(foo == local(y$id[2]), .colnames = "foo")
#> <SQL> ("foo" = 'b')

Created on 2023-10-06 with reprex v2.0.2