Skip to content

Commit

Permalink
Take UDFs out of build_expr and have Expression ensure Expression inputs
Browse files Browse the repository at this point in the history
  • Loading branch information
nealrichardson committed Oct 17, 2022
1 parent 73d1024 commit 0bf8d23
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 4 deletions.
2 changes: 1 addition & 1 deletion r/R/compute.R
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ register_scalar_function <- function(name, fun, in_type, out_type,
# register with dplyr binding (enables its use in mutate(), filter(), etc.)
register_binding(
name,
function(...) build_expr(name, ...),
function(...) Expression$create(name, ...),
update_cache = TRUE
)

Expand Down
8 changes: 7 additions & 1 deletion r/R/expression.R
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,13 @@ Expression$create <- function(function_name,
args = list(...),
options = empty_named_list()) {
assert_that(is.string(function_name))
assert_that(is_list_of(args, "Expression"), msg = "Expression arguments must be Expression objects")
# Make sure all inputs are Expressions
args <- lapply(args, function(x) {
if (!inherits(x, "Expression")) {
x <- Expression$scalar(x)
}
x
})
expr <- compute___expr__call(function_name, args, options)
if (length(args)) {
expr$schema <- unify_schemas(schemas = lapply(args, function(x) x$schema))
Expand Down
5 changes: 3 additions & 2 deletions r/tests/testthat/test-expression.R
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ test_that("C++ expressions", {
# Interprets that as a list type
expect_r6_class(f == c(1L, 2L), "Expression")

expect_error(
# Non-Expression inputs are wrapped in Expression$scalar()
expect_equal(
Expression$create("add", 1, 2),
"Expression arguments must be Expression objects"
Expression$create("add", Expression$scalar(1), Expression$scalar(2))
)
})

Expand Down

0 comments on commit 0bf8d23

Please sign in to comment.