nealrichardson commented on code in PR #13985:
URL: https://github.com/apache/arrow/pull/13985#discussion_r964809508
##########
r/R/expression.R:
##########
@@ -210,21 +213,20 @@ build_expr <- function(FUN,
}
if (FUN == "%in%") {
# Special-case %in%, which is different from the Array function name
+ value_set <- Array$create(args[[2]])
+ try(
+ value_set <- cast_or_parse(value_set, args[[1]]$type()),
+ silent = TRUE
+ )
+
expr <- Expression$create("is_in", args[[1]],
options = list(
- # If args[[2]] is already an Arrow object (like a scalar),
- # this wouldn't work
- value_set = Array$create(args[[2]]),
+ value_set = value_set,
skip_nulls = TRUE
)
)
} else {
- args <- lapply(args, function(x) {
- if (!inherits(x, "Expression")) {
- x <- Expression$scalar(x)
- }
- x
- })
+ args <- wrap_scalars(args, FUN)
Review Comment:
There's a blocklist rather than an allowlist, and binary_repeat is on it
(L285, below). If there are compute functions that don't work with this change,
we don't test them.
Do you think we should exclude UDFs from the type matching too?
For functions that do go through `build_expr()`, the way to skip the
type-matching logic is to wrap the value in `Expression$scalar()`. Only
non-Expressions are cast.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
