This is an automated email from the ASF dual-hosted git repository.
thisisnic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new c07de67a7c9 GH-37761: [R] Argument names ignored in schema supplied as
in_type argument to register_scalar_function() (#51324)
c07de67a7c9 is described below
commit c07de67a7c9a43c7c958ffeb08407b66c38df5b5
Author: Nic Crane <[email protected]>
AuthorDate: Fri Sep 18 10:38:09 2026 -0400
GH-37761: [R] Argument names ignored in schema supplied as in_type argument
to register_scalar_function() (#51324)
### Rationale for this change
No validation/warning makes it easy for incorrectly specified UDFs to
appear to work...but they don't!
### What changes are included in this PR?
Validation
### Are these changes tested?
Yeah
### Are there any user-facing changes?
I guess if they have incorrectly specified UDFs, yeah
## AI usage
All of it, we talked tho
* GitHub Issue: #37761
Authored-by: Nic Crane <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
---
r/R/udf.R | 36 ++++++++++++++++---
r/man/register_scalar_function.Rd | 5 ++-
r/tests/testthat/_snaps/udf.md | 12 +++++++
r/tests/testthat/test-udf.R | 76 +++++++++++++++++++++++++++++++++++++++
4 files changed, 124 insertions(+), 5 deletions(-)
diff --git a/r/R/udf.R b/r/R/udf.R
index ce7a911e0d9..64fae358f72 100644
--- a/r/R/udf.R
+++ b/r/R/udf.R
@@ -31,7 +31,10 @@
#' for functions with more than one argument. This signature will be used
#' to determine if this function is appropriate for a given set of arguments.
#' If this function is appropriate for more than one signature, pass a
-#' `list()` of the above.
+#' `list()` of the above. Arguments are passed to `fun` by position, so if
+#' the schema (or field) is named, the names must match the argument names
+#' of `fun` (after `context`). Fields that would be passed to `...` in `fun`
+#' can be named anything.
#' @param out_type A [DataType] of the output type or a function accepting
#' a single argument (`types`), which is a `list()` of [DataType]s. If a
#' function it must return a [DataType].
@@ -141,9 +144,12 @@ arrow_scalar_function <- function(fun, in_type, out_type,
auto_convert = FALSE)
abort("Can't register user-defined scalar function with 0 kernels")
}
+ # All kernels must have the same number of fields (RegisterScalarUDF enforces
+ # this later), so only the first needs comparing against fun
expected_n_args <- in_type[[1]]$num_fields + 1L
- fun_formals_have_dots <- any(names(formals(fun)) == "...")
- if (!fun_formals_have_dots && length(formals(fun)) != expected_n_args) {
+ fun_arg_names <- names(formals(fun))
+ fun_formals_have_dots <- any(fun_arg_names == "...")
+ if (!fun_formals_have_dots && length(fun_arg_names) != expected_n_args) {
abort(
sprintf(
paste0(
@@ -152,11 +158,33 @@ arrow_scalar_function <- function(fun, in_type, out_type,
auto_convert = FALSE)
"Did you forget to include `context` as the first argument?"
),
expected_n_args,
- length(formals(fun))
+ length(fun_arg_names)
)
)
}
+ # Arguments are passed to fun by position, so if the user named the
+ # fields in in_type, make sure those names line up with fun's arguments
+ # rather than silently ignoring them (GH-37761). Only the arguments
+ # explicitly named in fun (after `context` and before any `...`) are
+ # checked: fields beyond those are swallowed by `...` and can be named
+ # anything.
+ n_explicit_args <- match("...", fun_arg_names, nomatch =
length(fun_arg_names) + 1L) - 1L
+ explicit_arg_names <- fun_arg_names[seq_len(n_explicit_args)][-1]
+ in_type_names <- map(in_type, names)
+ mismatch <- map_lgl(in_type_names, function(nms) {
+ n_check <- min(length(nms), length(explicit_arg_names))
+ nms <- nms[seq_len(n_check)]
+ any(nzchar(nms) & nms != explicit_arg_names[seq_len(n_check)])
+ })
+ if (any(mismatch)) {
+ abort(c(
+ "Names in `in_type` must match the argument names of `fun` (after
`context`)",
+ x = paste0("`in_type` names: ",
oxford_paste(in_type_names[[which(mismatch)[1]]])),
+ x = paste0("`fun` argument names: ", oxford_paste(explicit_arg_names))
+ ))
+ }
+
structure(
list(
wrapper_fun = wrapper_fun,
diff --git a/r/man/register_scalar_function.Rd
b/r/man/register_scalar_function.Rd
index 6810740194c..fb43772add8 100644
--- a/r/man/register_scalar_function.Rd
+++ b/r/man/register_scalar_function.Rd
@@ -23,7 +23,10 @@ constructed with the expected output type via
\code{\link[=as_arrow_array]{as_ar
for functions with more than one argument. This signature will be used
to determine if this function is appropriate for a given set of arguments.
If this function is appropriate for more than one signature, pass a
-\code{list()} of the above.}
+\code{list()} of the above. Arguments are passed to \code{fun} by position, so
if
+the schema (or field) is named, the names must match the argument names
+of \code{fun} (after \code{context}). Fields that would be passed to
\code{...} in \code{fun}
+can be named anything.}
\item{out_type}{A \link{DataType} of the output type or a function accepting
a single argument (\code{types}), which is a \code{list()} of
\link{DataType}s. If a
diff --git a/r/tests/testthat/_snaps/udf.md b/r/tests/testthat/_snaps/udf.md
index 89506a7fbc2..be41a3f9690 100644
--- a/r/tests/testthat/_snaps/udf.md
+++ b/r/tests/testthat/_snaps/udf.md
@@ -2,3 +2,15 @@
fun is not a function
+# arrow_scalar_function() checks in_type names against fun arguments
+
+ Names in `in_type` must match the argument names of `fun` (after `context`)
+ x `in_type` names: "blah" and "aj"
+ x `fun` argument names: "x" and "y"
+
+---
+
+ Names in `in_type` must match the argument names of `fun` (after `context`)
+ x `in_type` names: "y"
+ x `fun` argument names: "x"
+
diff --git a/r/tests/testthat/test-udf.R b/r/tests/testthat/test-udf.R
index 2eadd87444b..428cc71f88a 100644
--- a/r/tests/testthat/test-udf.R
+++ b/r/tests/testthat/test-udf.R
@@ -323,3 +323,79 @@ test_that("head() on exec plan containing user-defined
functions", {
expect_equal(nrow(result), 11)
})
+
+test_that("arrow_scalar_function() checks in_type names against fun
arguments", {
+ # named schema with a different name than the argument
+ expect_snapshot_error(
+ arrow_scalar_function(
+ function(context, x, y) x,
+ schema(blah = int64(), aj = int64()),
+ int32()
+ )
+ )
+
+ # named field with a different name than the argument
+ expect_error(
+ arrow_scalar_function(
+ function(context, x) x,
+ field("blah", int64()),
+ int32()
+ ),
+ "must match the argument names"
+ )
+
+ # partial mismatch across multiple arguments
+ expect_error(
+ arrow_scalar_function(
+ function(context, x, y) x,
+ schema(x = int32(), b = int32()),
+ int32()
+ ),
+ "must match the argument names"
+ )
+
+ # mismatch in a later kernel when registering several at once: only the
+ # offending kernel's names are reported
+ expect_snapshot_error(
+ arrow_scalar_function(
+ function(context, x) x,
+ list(schema(x = int32()), schema(y = int32())),
+ int32()
+ )
+ )
+
+ # matching names, unnamed types, and `...` are all still accepted
+ expect_s3_class(
+ arrow_scalar_function(function(context, x) x, schema(x = int32()),
int32()),
+ "arrow_scalar_function"
+ )
+ expect_s3_class(
+ arrow_scalar_function(function(context, anything) anything, int32(),
int32()),
+ "arrow_scalar_function"
+ )
+ expect_s3_class(
+ arrow_scalar_function(function(...) NULL, schema(blah = int32()), int32()),
+ "arrow_scalar_function"
+ )
+})
+
+test_that("arrow_scalar_function() checks names of explicit arguments before
`...`", {
+ expect_error(
+ arrow_scalar_function(
+ function(context, x, ...) x,
+ schema(blah = int32()),
+ int32()
+ ),
+ "must match the argument names"
+ )
+
+ # fields beyond the explicit arguments go into `...` and can be named
anything
+ expect_s3_class(
+ arrow_scalar_function(
+ function(context, x, ...) x,
+ schema(x = int32(), blah = int32()),
+ int32()
+ ),
+ "arrow_scalar_function"
+ )
+})
