From 45b49493c9930d2004ec73c7f9c2a6f86dc125d9 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Sun, 13 Sep 2026 11:56:10 -0400 Subject: [PATCH 1/2] Add validation of udf names --- r/R/udf.R | 27 ++++++++++++++- r/man/register_scalar_function.Rd | 4 ++- r/tests/testthat/_snaps/udf.md | 6 ++++ r/tests/testthat/test-udf.R | 55 +++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) diff --git a/r/R/udf.R b/r/R/udf.R index ce7a911e0d97..23023243907e 100644 --- a/r/R/udf.R +++ b/r/R/udf.R @@ -31,7 +31,9 @@ #' 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`). #' @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]. @@ -157,6 +159,29 @@ arrow_scalar_function <- function(fun, in_type, out_type, auto_convert = FALSE) ) } + # 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) + if (!fun_formals_have_dots) { + fun_arg_names <- names(formals(fun))[-1] + # positions where a named field doesn't match the argument in that position + mismatched <- lapply(in_type, function(sig) { + nms <- names(sig) + same <- nms == fun_arg_names[seq_along(nms)] + same[is.na(same)] <- FALSE + which(nzchar(nms) & !same) + }) + first_bad <- which(lengths(mismatched) > 0)[1] + if (!is.na(first_bad)) { + pos <- mismatched[[first_bad]] + abort(c( + "Names in `in_type` must match the argument names of `fun` (after `context`)", + x = paste0("`in_type` names: ", oxford_paste(names(in_type[[first_bad]])[pos])), + x = paste0("`fun` argument names: ", oxford_paste(fun_arg_names[pos])) + )) + } + } + structure( list( wrapper_fun = wrapper_fun, diff --git a/r/man/register_scalar_function.Rd b/r/man/register_scalar_function.Rd index 6810740194c4..bcec0b2b85bc 100644 --- a/r/man/register_scalar_function.Rd +++ b/r/man/register_scalar_function.Rd @@ -23,7 +23,9 @@ 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}).} \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 89506a7fbc23..deaf54ae64b9 100644 --- a/r/tests/testthat/_snaps/udf.md +++ b/r/tests/testthat/_snaps/udf.md @@ -2,3 +2,9 @@ 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" + diff --git a/r/tests/testthat/test-udf.R b/r/tests/testthat/test-udf.R index 2eadd87444b5..e1743de104e0 100644 --- a/r/tests/testthat/test-udf.R +++ b/r/tests/testthat/test-udf.R @@ -323,3 +323,58 @@ 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 + expect_error( + arrow_scalar_function( + function(context, x) x, + list(schema(x = int32()), schema(y = int32())), + int32() + ), + "must match the argument names" + ) + + # 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" + ) +}) From 8b9e111f740ae963bd5b7e3f4bf96daca8a5e1b2 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Sun, 13 Sep 2026 12:03:38 -0400 Subject: [PATCH 2/2] Simplify comparison --- r/R/udf.R | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/r/R/udf.R b/r/R/udf.R index 23023243907e..89d3df15fd45 100644 --- a/r/R/udf.R +++ b/r/R/udf.R @@ -164,20 +164,19 @@ arrow_scalar_function <- function(fun, in_type, out_type, auto_convert = FALSE) # rather than silently ignoring them (GH-37761) if (!fun_formals_have_dots) { fun_arg_names <- names(formals(fun))[-1] - # positions where a named field doesn't match the argument in that position - mismatched <- lapply(in_type, function(sig) { - nms <- names(sig) - same <- nms == fun_arg_names[seq_along(nms)] - same[is.na(same)] <- FALSE - which(nzchar(nms) & !same) - }) - first_bad <- which(lengths(mismatched) > 0)[1] - if (!is.na(first_bad)) { - pos <- mismatched[[first_bad]] + in_type_names <- lapply(in_type, names) + mismatch <- vapply( + in_type_names, + function(nms) { + isTRUE(any(nzchar(nms) & nms != fun_arg_names[seq_along(nms)])) + }, + logical(1) + ) + 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(names(in_type[[first_bad]])[pos])), - x = paste0("`fun` argument names: ", oxford_paste(fun_arg_names[pos])) + x = paste0("`in_type` names: ", oxford_paste(unlist(in_type_names))), + x = paste0("`fun` argument names: ", oxford_paste(fun_arg_names)) )) } }