Jefffrey commented on code in PR #18019:
URL: https://github.com/apache/datafusion/pull/18019#discussion_r2442705784
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -996,13 +1159,119 @@ impl Signature {
},
),
volatility,
+ parameter_names: None,
}
}
/// Specialized [Signature] for ArrayEmpty and similar functions.
pub fn array(volatility: Volatility) -> Self {
Signature::arrays(1, Some(ListCoercion::FixedSizedListToList),
volatility)
}
+
+ /// Add parameter names to this signature, enabling named argument
notation.
+ ///
+ /// # Example
+ /// ```
+ /// # use datafusion_expr_common::signature::{Signature, Volatility};
+ /// # use arrow::datatypes::DataType;
+ /// let sig = Signature::exact(vec![DataType::Int32, DataType::Utf8],
Volatility::Immutable)
+ /// .with_parameter_names(vec!["count".to_string(),
"name".to_string()]);
+ /// ```
+ ///
+ /// # Errors
+ /// Returns an error if the number of parameter names doesn't match the
signature's arity.
+ /// For signatures with variable arity (e.g., `Variadic`, `VariadicAny`),
parameter names
+ /// cannot be specified.
+ pub fn with_parameter_names(
+ mut self,
+ names: Vec<String>,
+ ) -> datafusion_common::Result<Self> {
+ // Validate that the number of names matches the signature
+ self.validate_parameter_names(&names)?;
+ self.parameter_names = Some(names);
+ Ok(self)
+ }
+
+ /// Validate that parameter names are compatible with this signature
+ fn validate_parameter_names(
+ &self,
+ names: &[String],
+ ) -> datafusion_common::Result<()> {
+ // Get expected argument count from the type signature
+ let expected_count = match &self.type_signature {
Review Comment:
Maybe we should pull this expected count logic out of this function and put
it as a function on `TypeSignature`:
```rust
let expected_count = self.type_signature.arity()
```
Though would need handling for variadic (maybe return an enum?)
##########
datafusion/expr/src/udf.rs:
##########
@@ -957,3 +949,284 @@ mod tests {
hasher.finish()
}
}
+
+/// Argument resolution logic for named function parameters
+pub mod arguments {
+ use crate::Expr;
+ use datafusion_common::{plan_err, Result};
+
+ /// Resolves function arguments, handling named and positional notation.
+ ///
+ /// This function validates and reorders arguments to match the function's
parameter names
+ /// when named arguments are used.
+ ///
+ /// # Rules
+ /// - All positional arguments must come before named arguments
+ /// - Named arguments can be in any order after positional arguments
+ /// - All parameter names must match the provided parameter_names
+ /// - No duplicate parameter names allowed
+ ///
+ /// # Arguments
+ /// * `param_names` - The function's parameter names in order
+ /// * `args` - The argument expressions
+ /// * `arg_names` - Optional parameter name for each argument
+ ///
+ /// # Returns
+ /// A vector of expressions in the correct order matching the parameter
names
+ ///
+ /// # Examples
+ /// ```text
+ /// Given parameters ["a", "b", "c"]
+ /// And call: func(10, c => 30, b => 20)
+ /// Returns: [Expr(10), Expr(20), Expr(30)]
+ /// ```
+ pub fn resolve_function_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Validate that arg_names length matches args length
+ if args.len() != arg_names.len() {
+ return plan_err!(
+ "Internal error: args length ({}) != arg_names length ({})",
+ args.len(),
+ arg_names.len()
+ );
+ }
+
+ // Check if all arguments are positional (fast path)
+ if arg_names.iter().all(|name| name.is_none()) {
+ return Ok(args);
+ }
+
+ // Validate mixed positional and named arguments
+ validate_argument_order(&arg_names)?;
+
+ // Validate and reorder named arguments
+ reorder_named_arguments(param_names, args, arg_names)
+ }
+
+ /// Validates that positional arguments come before named arguments
+ fn validate_argument_order(arg_names: &[Option<String>]) -> Result<()> {
+ let mut seen_named = false;
+ for (i, arg_name) in arg_names.iter().enumerate() {
+ match arg_name {
+ Some(_) => seen_named = true,
+ None if seen_named => {
+ return plan_err!(
+ "Positional argument at position {} follows named
argument. \
+ All positional arguments must come before named
arguments.",
+ i
+ );
+ }
+ None => {}
+ }
+ }
+ Ok(())
+ }
+
+ /// Reorders arguments based on named parameters to match signature order
+ fn reorder_named_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Count positional vs named arguments
+ let positional_count = arg_names.iter().filter(|n|
n.is_none()).count();
+
+ // Capture args length before consuming the vector
+ let args_len = args.len();
+
+ // Create a result vector with the expected size
+ let expected_arg_count = param_names.len();
+ let mut result: Vec<Option<Expr>> = vec![None; expected_arg_count];
+
+ // Track which parameters have been assigned
+ let mut assigned = vec![false; expected_arg_count];
+
+ // Process all arguments (both positional and named)
+ for (i, (arg, arg_name)) in
args.into_iter().zip(arg_names).enumerate() {
+ if let Some(name) = arg_name {
+ // Named argument - find its position in param_names
+ let param_index =
+ param_names.iter().position(|p| p == &name).ok_or_else(|| {
+ datafusion_common::plan_datafusion_err!(
+ "Unknown parameter name '{}'. Valid parameters
are: [{}]",
+ name,
+ param_names.join(", ")
+ )
+ })?;
+
+ // Check if this parameter was already assigned
+ if assigned[param_index] {
+ return plan_err!("Parameter '{}' specified multiple
times", name);
+ }
+
+ result[param_index] = Some(arg);
+ assigned[param_index] = true;
+ } else {
+ // Positional argument - place at current position
+ if i >= expected_arg_count {
+ return plan_err!(
+ "Too many positional arguments: expected at most {},
got {}",
+ expected_arg_count,
+ positional_count
+ );
+ }
Review Comment:
I feel this is something we should check upfront and not in the for loop?
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -996,13 +1159,119 @@ impl Signature {
},
),
volatility,
+ parameter_names: None,
}
}
/// Specialized [Signature] for ArrayEmpty and similar functions.
pub fn array(volatility: Volatility) -> Self {
Signature::arrays(1, Some(ListCoercion::FixedSizedListToList),
volatility)
}
+
+ /// Add parameter names to this signature, enabling named argument
notation.
+ ///
+ /// # Example
+ /// ```
+ /// # use datafusion_expr_common::signature::{Signature, Volatility};
+ /// # use arrow::datatypes::DataType;
+ /// let sig = Signature::exact(vec![DataType::Int32, DataType::Utf8],
Volatility::Immutable)
+ /// .with_parameter_names(vec!["count".to_string(),
"name".to_string()]);
+ /// ```
+ ///
+ /// # Errors
+ /// Returns an error if the number of parameter names doesn't match the
signature's arity.
+ /// For signatures with variable arity (e.g., `Variadic`, `VariadicAny`),
parameter names
+ /// cannot be specified.
+ pub fn with_parameter_names(
+ mut self,
+ names: Vec<String>,
+ ) -> datafusion_common::Result<Self> {
+ // Validate that the number of names matches the signature
+ self.validate_parameter_names(&names)?;
+ self.parameter_names = Some(names);
+ Ok(self)
+ }
Review Comment:
```suggestion
pub fn with_parameter_names(
mut self,
names: Vec<impl Into<String>>,
) -> datafusion_common::Result<Self> {
let names =
names.into_iter().map(Into::into).collect::<Vec<String>>();
// Validate that the number of names matches the signature
self.validate_parameter_names(&names)?;
self.parameter_names = Some(names);
Ok(self)
}
```
Would allow usage like `.with_parameter_names(vec!["a", "b"])` to make it
slightly nicer to use
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -996,13 +1159,119 @@ impl Signature {
},
),
volatility,
+ parameter_names: None,
}
}
/// Specialized [Signature] for ArrayEmpty and similar functions.
pub fn array(volatility: Volatility) -> Self {
Signature::arrays(1, Some(ListCoercion::FixedSizedListToList),
volatility)
}
+
+ /// Add parameter names to this signature, enabling named argument
notation.
+ ///
+ /// # Example
+ /// ```
+ /// # use datafusion_expr_common::signature::{Signature, Volatility};
+ /// # use arrow::datatypes::DataType;
+ /// let sig = Signature::exact(vec![DataType::Int32, DataType::Utf8],
Volatility::Immutable)
+ /// .with_parameter_names(vec!["count".to_string(),
"name".to_string()]);
+ /// ```
+ ///
+ /// # Errors
+ /// Returns an error if the number of parameter names doesn't match the
signature's arity.
+ /// For signatures with variable arity (e.g., `Variadic`, `VariadicAny`),
parameter names
+ /// cannot be specified.
+ pub fn with_parameter_names(
+ mut self,
+ names: Vec<String>,
+ ) -> datafusion_common::Result<Self> {
+ // Validate that the number of names matches the signature
+ self.validate_parameter_names(&names)?;
+ self.parameter_names = Some(names);
+ Ok(self)
+ }
+
+ /// Validate that parameter names are compatible with this signature
+ fn validate_parameter_names(
+ &self,
+ names: &[String],
+ ) -> datafusion_common::Result<()> {
+ // Get expected argument count from the type signature
+ let expected_count = match &self.type_signature {
+ TypeSignature::Exact(types) => Some(types.len()),
+ TypeSignature::Uniform(count, _) => Some(*count),
+ TypeSignature::Numeric(count) => Some(*count),
+ TypeSignature::String(count) => Some(*count),
+ TypeSignature::Comparable(count) => Some(*count),
+ TypeSignature::Any(count) => Some(*count),
+ TypeSignature::Coercible(types) => Some(types.len()),
+ TypeSignature::Nullary => Some(0),
+ TypeSignature::ArraySignature(ArrayFunctionSignature::Array {
+ arguments,
+ ..
+ }) => Some(arguments.len()),
+
TypeSignature::ArraySignature(ArrayFunctionSignature::RecursiveArray) => {
+ Some(1)
+ }
+ TypeSignature::ArraySignature(ArrayFunctionSignature::MapArray) =>
Some(1),
+ // For OneOf, get the maximum arity from all variants
+ TypeSignature::OneOf(variants) => {
+ // Get max arity from all variants
+ let max_arity = variants
+ .iter()
+ .filter_map(|v| match v {
+ TypeSignature::Any(count)
+ | TypeSignature::Uniform(count, _)
+ | TypeSignature::Numeric(count)
+ | TypeSignature::String(count)
+ | TypeSignature::Comparable(count) => Some(*count),
+ TypeSignature::Exact(types) => Some(types.len()),
+ TypeSignature::Coercible(types) => Some(types.len()),
+ TypeSignature::Nullary => Some(0),
+ _ => None,
+ })
+ .max();
+ max_arity
+ }
+ // Variable arity signatures cannot have parameter names
+ TypeSignature::Variadic(_)
+ | TypeSignature::VariadicAny
+ | TypeSignature::UserDefined => None,
+ };
+
+ if let Some(expected) = expected_count {
+ if names.len() != expected {
+ return datafusion_common::plan_err!(
+ "Parameter names count ({}) does not match signature arity
({})",
+ names.len(),
+ expected
+ );
+ }
+ } else {
+ // For UserDefined signatures, allow parameter names
+ // The function implementer is responsible for validating the
names match the actual arguments
+ if !matches!(self.type_signature, TypeSignature::UserDefined) {
+ return datafusion_common::plan_err!(
Review Comment:
We should clean up some of this code by importing this instead of qualifying
everywhere:
```
use datafusion_common::plan_err;
```
I see it for other types too
##########
datafusion/expr/src/udf.rs:
##########
@@ -957,3 +949,284 @@ mod tests {
hasher.finish()
}
}
+
+/// Argument resolution logic for named function parameters
+pub mod arguments {
+ use crate::Expr;
+ use datafusion_common::{plan_err, Result};
+
+ /// Resolves function arguments, handling named and positional notation.
+ ///
+ /// This function validates and reorders arguments to match the function's
parameter names
+ /// when named arguments are used.
+ ///
+ /// # Rules
+ /// - All positional arguments must come before named arguments
+ /// - Named arguments can be in any order after positional arguments
+ /// - All parameter names must match the provided parameter_names
+ /// - No duplicate parameter names allowed
+ ///
+ /// # Arguments
+ /// * `param_names` - The function's parameter names in order
+ /// * `args` - The argument expressions
+ /// * `arg_names` - Optional parameter name for each argument
+ ///
+ /// # Returns
+ /// A vector of expressions in the correct order matching the parameter
names
+ ///
+ /// # Examples
+ /// ```text
+ /// Given parameters ["a", "b", "c"]
+ /// And call: func(10, c => 30, b => 20)
+ /// Returns: [Expr(10), Expr(20), Expr(30)]
+ /// ```
+ pub fn resolve_function_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Validate that arg_names length matches args length
+ if args.len() != arg_names.len() {
+ return plan_err!(
+ "Internal error: args length ({}) != arg_names length ({})",
+ args.len(),
+ arg_names.len()
+ );
+ }
+
+ // Check if all arguments are positional (fast path)
+ if arg_names.iter().all(|name| name.is_none()) {
+ return Ok(args);
+ }
+
+ // Validate mixed positional and named arguments
+ validate_argument_order(&arg_names)?;
+
+ // Validate and reorder named arguments
+ reorder_named_arguments(param_names, args, arg_names)
+ }
+
+ /// Validates that positional arguments come before named arguments
+ fn validate_argument_order(arg_names: &[Option<String>]) -> Result<()> {
+ let mut seen_named = false;
+ for (i, arg_name) in arg_names.iter().enumerate() {
+ match arg_name {
+ Some(_) => seen_named = true,
+ None if seen_named => {
+ return plan_err!(
+ "Positional argument at position {} follows named
argument. \
+ All positional arguments must come before named
arguments.",
+ i
+ );
+ }
+ None => {}
+ }
+ }
+ Ok(())
+ }
+
+ /// Reorders arguments based on named parameters to match signature order
+ fn reorder_named_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Count positional vs named arguments
+ let positional_count = arg_names.iter().filter(|n|
n.is_none()).count();
+
+ // Capture args length before consuming the vector
+ let args_len = args.len();
+
+ // Create a result vector with the expected size
+ let expected_arg_count = param_names.len();
+ let mut result: Vec<Option<Expr>> = vec![None; expected_arg_count];
+
+ // Track which parameters have been assigned
+ let mut assigned = vec![false; expected_arg_count];
+
+ // Process all arguments (both positional and named)
+ for (i, (arg, arg_name)) in
args.into_iter().zip(arg_names).enumerate() {
+ if let Some(name) = arg_name {
+ // Named argument - find its position in param_names
+ let param_index =
+ param_names.iter().position(|p| p == &name).ok_or_else(|| {
+ datafusion_common::plan_datafusion_err!(
+ "Unknown parameter name '{}'. Valid parameters
are: [{}]",
+ name,
+ param_names.join(", ")
+ )
+ })?;
+
+ // Check if this parameter was already assigned
+ if assigned[param_index] {
+ return plan_err!("Parameter '{}' specified multiple
times", name);
+ }
+
+ result[param_index] = Some(arg);
+ assigned[param_index] = true;
+ } else {
+ // Positional argument - place at current position
+ if i >= expected_arg_count {
+ return plan_err!(
+ "Too many positional arguments: expected at most {},
got {}",
+ expected_arg_count,
+ positional_count
+ );
+ }
+ result[i] = Some(arg);
+ assigned[i] = true;
+ }
+ }
+
+ // Check if all required parameters were provided
+ // Only require parameters up to the number of arguments provided
(supports optional parameters)
+ let required_count = args_len;
+ for i in 0..required_count {
+ if !assigned[i] {
+ return plan_err!("Missing required parameter '{}'",
param_names[i]);
+ }
+ }
+
+ // Return only the assigned parameters (handles optional trailing
parameters)
+ Ok(result
+ .into_iter()
+ .take(required_count)
+ .map(|e| e.unwrap())
Review Comment:
```suggestion
.flatten()
```
##########
datafusion/expr/src/udf.rs:
##########
@@ -957,3 +949,284 @@ mod tests {
hasher.finish()
}
}
+
+/// Argument resolution logic for named function parameters
+pub mod arguments {
+ use crate::Expr;
+ use datafusion_common::{plan_err, Result};
+
+ /// Resolves function arguments, handling named and positional notation.
+ ///
+ /// This function validates and reorders arguments to match the function's
parameter names
+ /// when named arguments are used.
+ ///
+ /// # Rules
+ /// - All positional arguments must come before named arguments
+ /// - Named arguments can be in any order after positional arguments
+ /// - All parameter names must match the provided parameter_names
+ /// - No duplicate parameter names allowed
+ ///
+ /// # Arguments
+ /// * `param_names` - The function's parameter names in order
+ /// * `args` - The argument expressions
+ /// * `arg_names` - Optional parameter name for each argument
+ ///
+ /// # Returns
+ /// A vector of expressions in the correct order matching the parameter
names
+ ///
+ /// # Examples
+ /// ```text
+ /// Given parameters ["a", "b", "c"]
+ /// And call: func(10, c => 30, b => 20)
+ /// Returns: [Expr(10), Expr(20), Expr(30)]
+ /// ```
+ pub fn resolve_function_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Validate that arg_names length matches args length
+ if args.len() != arg_names.len() {
+ return plan_err!(
+ "Internal error: args length ({}) != arg_names length ({})",
+ args.len(),
+ arg_names.len()
+ );
+ }
+
+ // Check if all arguments are positional (fast path)
+ if arg_names.iter().all(|name| name.is_none()) {
+ return Ok(args);
+ }
+
+ // Validate mixed positional and named arguments
+ validate_argument_order(&arg_names)?;
+
+ // Validate and reorder named arguments
+ reorder_named_arguments(param_names, args, arg_names)
+ }
+
+ /// Validates that positional arguments come before named arguments
+ fn validate_argument_order(arg_names: &[Option<String>]) -> Result<()> {
+ let mut seen_named = false;
+ for (i, arg_name) in arg_names.iter().enumerate() {
+ match arg_name {
+ Some(_) => seen_named = true,
+ None if seen_named => {
+ return plan_err!(
+ "Positional argument at position {} follows named
argument. \
+ All positional arguments must come before named
arguments.",
+ i
+ );
+ }
+ None => {}
+ }
+ }
+ Ok(())
+ }
+
+ /// Reorders arguments based on named parameters to match signature order
+ fn reorder_named_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Count positional vs named arguments
+ let positional_count = arg_names.iter().filter(|n|
n.is_none()).count();
+
+ // Capture args length before consuming the vector
+ let args_len = args.len();
+
+ // Create a result vector with the expected size
+ let expected_arg_count = param_names.len();
+ let mut result: Vec<Option<Expr>> = vec![None; expected_arg_count];
+
+ // Track which parameters have been assigned
+ let mut assigned = vec![false; expected_arg_count];
+
+ // Process all arguments (both positional and named)
+ for (i, (arg, arg_name)) in
args.into_iter().zip(arg_names).enumerate() {
+ if let Some(name) = arg_name {
+ // Named argument - find its position in param_names
+ let param_index =
+ param_names.iter().position(|p| p == &name).ok_or_else(|| {
+ datafusion_common::plan_datafusion_err!(
+ "Unknown parameter name '{}'. Valid parameters
are: [{}]",
+ name,
+ param_names.join(", ")
+ )
+ })?;
+
+ // Check if this parameter was already assigned
+ if assigned[param_index] {
+ return plan_err!("Parameter '{}' specified multiple
times", name);
+ }
+
+ result[param_index] = Some(arg);
+ assigned[param_index] = true;
Review Comment:
We can probably remove `assigned` vec and rely on checking `result` if it is
Some or None
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -486,6 +486,145 @@ impl TypeSignature {
}
}
+ /// Return string representation of the function signature with parameter
names.
+ ///
+ /// This method is similar to [`Self::to_string_repr`] but uses parameter
names
+ /// instead of types when available. This is useful for generating more
helpful
+ /// error messages.
+ ///
+ /// # Arguments
+ /// * `parameter_names` - Optional slice of parameter names. When
provided, these
+ /// names will be used instead of type names in the output.
+ ///
+ /// # Examples
+ /// ```
+ /// # use datafusion_expr_common::signature::TypeSignature;
+ /// # use arrow::datatypes::DataType;
+ /// let sig = TypeSignature::Exact(vec![DataType::Int32, DataType::Utf8]);
+ ///
+ /// // Without names: shows types
+ /// assert_eq!(sig.to_string_repr_with_names(None), vec!["Int32, Utf8"]);
+ ///
+ /// // With names: shows parameter names
+ /// assert_eq!(
+ /// sig.to_string_repr_with_names(Some(&["id".to_string(),
"name".to_string()])),
+ /// vec!["id, name"]
+ /// );
+ /// ```
+ pub fn to_string_repr_with_names(
+ &self,
+ parameter_names: Option<&[String]>,
+ ) -> Vec<String> {
+ match self {
+ TypeSignature::Exact(types) => {
+ if let Some(names) = parameter_names {
+ vec![names
+ .iter()
+ .take(types.len())
Review Comment:
This `take` (and in other branches too) feels a little odd; it feels this is
something we should already know (that the lengths match) and thus we don't
need to constantly check it, or something we check up front, instead of
implicitly assuming here. Or do I misunderstand the purpose?
##########
datafusion/expr/src/udf.rs:
##########
@@ -957,3 +949,284 @@ mod tests {
hasher.finish()
}
}
+
+/// Argument resolution logic for named function parameters
+pub mod arguments {
+ use crate::Expr;
+ use datafusion_common::{plan_err, Result};
+
+ /// Resolves function arguments, handling named and positional notation.
+ ///
+ /// This function validates and reorders arguments to match the function's
parameter names
+ /// when named arguments are used.
+ ///
+ /// # Rules
+ /// - All positional arguments must come before named arguments
+ /// - Named arguments can be in any order after positional arguments
+ /// - All parameter names must match the provided parameter_names
+ /// - No duplicate parameter names allowed
+ ///
+ /// # Arguments
+ /// * `param_names` - The function's parameter names in order
+ /// * `args` - The argument expressions
+ /// * `arg_names` - Optional parameter name for each argument
+ ///
+ /// # Returns
+ /// A vector of expressions in the correct order matching the parameter
names
+ ///
+ /// # Examples
+ /// ```text
+ /// Given parameters ["a", "b", "c"]
+ /// And call: func(10, c => 30, b => 20)
+ /// Returns: [Expr(10), Expr(20), Expr(30)]
+ /// ```
+ pub fn resolve_function_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Validate that arg_names length matches args length
+ if args.len() != arg_names.len() {
+ return plan_err!(
+ "Internal error: args length ({}) != arg_names length ({})",
+ args.len(),
+ arg_names.len()
+ );
+ }
+
+ // Check if all arguments are positional (fast path)
+ if arg_names.iter().all(|name| name.is_none()) {
+ return Ok(args);
+ }
+
+ // Validate mixed positional and named arguments
+ validate_argument_order(&arg_names)?;
+
+ // Validate and reorder named arguments
+ reorder_named_arguments(param_names, args, arg_names)
+ }
+
+ /// Validates that positional arguments come before named arguments
+ fn validate_argument_order(arg_names: &[Option<String>]) -> Result<()> {
+ let mut seen_named = false;
+ for (i, arg_name) in arg_names.iter().enumerate() {
+ match arg_name {
+ Some(_) => seen_named = true,
+ None if seen_named => {
+ return plan_err!(
+ "Positional argument at position {} follows named
argument. \
+ All positional arguments must come before named
arguments.",
+ i
+ );
+ }
+ None => {}
+ }
+ }
+ Ok(())
+ }
+
+ /// Reorders arguments based on named parameters to match signature order
+ fn reorder_named_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Count positional vs named arguments
+ let positional_count = arg_names.iter().filter(|n|
n.is_none()).count();
+
+ // Capture args length before consuming the vector
+ let args_len = args.len();
+
+ // Create a result vector with the expected size
+ let expected_arg_count = param_names.len();
+ let mut result: Vec<Option<Expr>> = vec![None; expected_arg_count];
+
+ // Track which parameters have been assigned
+ let mut assigned = vec![false; expected_arg_count];
+
+ // Process all arguments (both positional and named)
+ for (i, (arg, arg_name)) in
args.into_iter().zip(arg_names).enumerate() {
+ if let Some(name) = arg_name {
+ // Named argument - find its position in param_names
+ let param_index =
+ param_names.iter().position(|p| p == &name).ok_or_else(|| {
Review Comment:
Should we use a hashmap for param names instead of iterating the vec each
time?
##########
datafusion/expr/src/udf.rs:
##########
@@ -957,3 +949,284 @@ mod tests {
hasher.finish()
}
}
+
+/// Argument resolution logic for named function parameters
+pub mod arguments {
+ use crate::Expr;
+ use datafusion_common::{plan_err, Result};
+
+ /// Resolves function arguments, handling named and positional notation.
+ ///
+ /// This function validates and reorders arguments to match the function's
parameter names
+ /// when named arguments are used.
+ ///
+ /// # Rules
+ /// - All positional arguments must come before named arguments
+ /// - Named arguments can be in any order after positional arguments
+ /// - All parameter names must match the provided parameter_names
+ /// - No duplicate parameter names allowed
+ ///
+ /// # Arguments
+ /// * `param_names` - The function's parameter names in order
+ /// * `args` - The argument expressions
+ /// * `arg_names` - Optional parameter name for each argument
+ ///
+ /// # Returns
+ /// A vector of expressions in the correct order matching the parameter
names
+ ///
+ /// # Examples
+ /// ```text
+ /// Given parameters ["a", "b", "c"]
+ /// And call: func(10, c => 30, b => 20)
+ /// Returns: [Expr(10), Expr(20), Expr(30)]
+ /// ```
+ pub fn resolve_function_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Validate that arg_names length matches args length
+ if args.len() != arg_names.len() {
+ return plan_err!(
+ "Internal error: args length ({}) != arg_names length ({})",
+ args.len(),
+ arg_names.len()
+ );
+ }
+
+ // Check if all arguments are positional (fast path)
+ if arg_names.iter().all(|name| name.is_none()) {
+ return Ok(args);
+ }
+
+ // Validate mixed positional and named arguments
+ validate_argument_order(&arg_names)?;
+
+ // Validate and reorder named arguments
+ reorder_named_arguments(param_names, args, arg_names)
+ }
+
+ /// Validates that positional arguments come before named arguments
+ fn validate_argument_order(arg_names: &[Option<String>]) -> Result<()> {
+ let mut seen_named = false;
+ for (i, arg_name) in arg_names.iter().enumerate() {
+ match arg_name {
+ Some(_) => seen_named = true,
+ None if seen_named => {
+ return plan_err!(
+ "Positional argument at position {} follows named
argument. \
+ All positional arguments must come before named
arguments.",
+ i
+ );
+ }
+ None => {}
+ }
+ }
+ Ok(())
+ }
+
+ /// Reorders arguments based on named parameters to match signature order
+ fn reorder_named_arguments(
+ param_names: &[String],
+ args: Vec<Expr>,
+ arg_names: Vec<Option<String>>,
+ ) -> Result<Vec<Expr>> {
+ // Count positional vs named arguments
+ let positional_count = arg_names.iter().filter(|n|
n.is_none()).count();
+
+ // Capture args length before consuming the vector
+ let args_len = args.len();
+
+ // Create a result vector with the expected size
+ let expected_arg_count = param_names.len();
+ let mut result: Vec<Option<Expr>> = vec![None; expected_arg_count];
+
+ // Track which parameters have been assigned
+ let mut assigned = vec![false; expected_arg_count];
+
+ // Process all arguments (both positional and named)
Review Comment:
Can we remove these LLM-like comments? Having too much of them adds
unnecessary verbosity
##########
docs/source/library-user-guide/functions/adding-udfs.md:
##########
@@ -586,6 +586,119 @@ For async UDF implementation details, see
[`async_udf.rs`](https://github.com/ap
[`process_scalar_func_inputs`]:
https://docs.rs/datafusion/latest/datafusion/physical_expr/functions/fn.process_scalar_func_inputs.html
[`advanced_udf.rs`]:
https://github.com/apache/datafusion/blob/main/datafusion-examples/examples/advanced_udf.rs
+## Named Arguments
+
+DataFusion supports PostgreSQL-style named arguments for scalar functions,
allowing you to pass arguments by parameter name:
+
+```sql
+SELECT substr(str => 'hello', start_pos => 2, length => 3);
+```
+
+Named arguments can be mixed with positional arguments, but positional
arguments must come first:
+
+```sql
+SELECT substr('hello', start_pos => 2, length => 3); -- Valid
+```
+
+### Implementing Functions with Named Arguments
+
+To support named arguments in your UDF, add parameter names to your function's
signature using `.with_parameter_names()`:
+
+```rust
+# use arrow::datatypes::DataType;
+# use datafusion_expr::{Signature, Volatility};
+#
+# #[derive(Debug)]
+# struct MyFunction {
+# signature: Signature,
+# }
+#
+impl MyFunction {
+ fn new() -> Self {
+ Self {
+ signature: Signature::uniform(
+ 2,
+ vec![DataType::Float64],
+ Volatility::Immutable
+ )
+ .with_parameter_names(vec![
+ "base".to_string(),
+ "exponent".to_string()
+ ])
+ .expect("valid parameter names"),
+ }
+ }
+}
+```
+
+The parameter names should match the order of arguments in your function's
signature. DataFusion automatically resolves named arguments to the correct
positional order before invoking your function.
+
+### Example
+
+```rust
+# use std::sync::Arc;
+# use std::any::Any;
+# use arrow::datatypes::DataType;
+# use datafusion_common::Result;
+# use datafusion_expr::{ColumnarValue, ScalarFunctionArgs, Signature,
Volatility};
+# use datafusion_expr::ScalarUDFImpl;
+
+#[derive(Debug, PartialEq, Eq, Hash)]
+struct PowerFunction {
+ signature: Signature,
+}
+
+impl PowerFunction {
+ fn new() -> Self {
+ Self {
+ signature: Signature::uniform(
+ 2,
+ vec![DataType::Float64],
+ Volatility::Immutable
+ )
+ .with_parameter_names(vec![
+ "base".to_string(),
+ "exponent".to_string()
+ ])
+ .expect("valid parameter names"),
+ }
+ }
+}
+
+impl ScalarUDFImpl for PowerFunction {
+ fn as_any(&self) -> &dyn Any { self }
+ fn name(&self) -> &str { "power" }
+ fn signature(&self) -> &Signature { &self.signature }
+
+ fn return_type(&self, _args: &[DataType]) -> Result<DataType> {
+ Ok(DataType::Float64)
+ }
+
+ fn invoke_with_args(&self, _args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
+ // Your implementation - arguments are in correct positional order
+ unimplemented!()
+ }
+}
+```
+
+Once registered, users can call your function with named arguments:
+
+```sql
+SELECT power(base => 2.0, exponent => 3.0);
+SELECT power(2.0, exponent => 3.0);
+```
+
+### Error Messages
+
+When a function call fails due to incorrect arguments, DataFusion will show
the parameter names in error messages to help users:
+
+```text
+No function matches the given name and argument types substr(Utf8).
+ Candidate functions:
+ substr(str, start_pos)
+ substr(str, start_pos, length)
Review Comment:
This is nice; though does that mean we lose type information in the
candidate functions?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
