Jefffrey commented on code in PR #19308:
URL: https://github.com/apache/datafusion/pull/19308#discussion_r2636876410
##########
datafusion/sqllogictest/test_files/named_arguments.slt:
##########
@@ -269,3 +269,80 @@ SELECT row_number(value => 1) OVER (ORDER BY id) FROM
window_test;
# Cleanup
statement ok
DROP TABLE window_test;
+
+#############
+## Test UDF with Many Optional Parameters (test_optional_params - sums p1
through p7)
Review Comment:
Do these SLT tests duplicate the same unit tests in `arguments`?
##########
datafusion/expr/src/arguments.rs:
##########
@@ -274,20 +276,254 @@ mod tests {
}
#[test]
- fn test_missing_required_parameter() {
+ fn test_skip_middle_optional_parameter() {
let param_names = vec!["a".to_string(), "b".to_string(),
"c".to_string()];
- // Call with: func(a => 1, c => 3.0) - missing 'b'
+ // Call with: func(a => 1, c => 3.0) - skipping 'b', should fill with
NULL
let args = vec![lit(1), lit(3.0)];
let arg_names = vec![Some("a".to_string()), Some("c".to_string())];
+ let result = resolve_function_arguments(¶m_names, args,
arg_names).unwrap();
+
+ assert_result_pattern(&result, &[Some(lit(1)), None, Some(lit(3.0))]);
+ }
+
+ #[test]
+ fn test_skip_multiple_middle_parameters() {
+ let param_names = vec![
+ "a".to_string(),
+ "b".to_string(),
+ "c".to_string(),
+ "d".to_string(),
+ "e".to_string(),
+ ];
+
+ // Call with: func(a => 1, e => 5) - skipping b, c, d
+ let args = vec![lit(1), lit(5)];
+ let arg_names = vec![Some("a".to_string()), Some("e".to_string())];
+
+ let result = resolve_function_arguments(¶m_names, args,
arg_names).unwrap();
+
+ assert_result_pattern(&result, &[Some(lit(1)), None, None, None,
Some(lit(5))]);
+ }
+
+ #[test]
+ fn test_skip_first_parameters() {
+ let param_names = vec!["a".to_string(), "b".to_string(),
"c".to_string()];
+
+ // Call with: func(c => 3.0) - skipping a, b
+ let args = vec![lit(3.0)];
+ let arg_names = vec![Some("c".to_string())];
+
+ let result = resolve_function_arguments(¶m_names, args,
arg_names).unwrap();
+
+ assert_result_pattern(&result, &[None, None, Some(lit(3.0))]);
+ }
+
+ #[test]
+ fn test_mixed_positional_and_skipped_named() {
+ let param_names = vec![
+ "a".to_string(),
+ "b".to_string(),
+ "c".to_string(),
+ "d".to_string(),
+ ];
+
+ // Call with: func(1, 2, d => 4) - positional a, b, skip c, named d
+ let args = vec![lit(1), lit(2), lit(4)];
+ let arg_names = vec![None, None, Some("d".to_string())];
+
+ let result = resolve_function_arguments(¶m_names, args,
arg_names).unwrap();
+
+ assert_result_pattern(&result, &[Some(lit(1)), Some(lit(2)), None,
Some(lit(4))]);
+ }
+
+ #[test]
+ fn test_alternating_filled_and_skipped() {
Review Comment:
I feel this test is the same as `test_sparse_parameters`, can probably
remove this
##########
datafusion/sqllogictest/src/test_context.rs:
##########
@@ -567,3 +518,139 @@ fn register_async_abs_udf(ctx: &SessionContext) {
let udf = AsyncScalarUDF::new(Arc::new(async_abs));
ctx.register_udf(udf.into_scalar_udf());
}
+
+/// Creates a test UDF with many optional parameters to test named argument
skipping
+fn create_optional_params_test_udf() -> ScalarUDF {
+ use datafusion::arrow::array::Int64Array;
+ use datafusion::common::types::{logical_int64, NativeType};
+ use datafusion::logical_expr::{Coercion, TypeSignature};
+
+ #[derive(Debug, PartialEq, Eq, Hash)]
+ struct TestOptionalParamsUDF {
+ signature: Signature,
+ }
+
+ impl TestOptionalParamsUDF {
+ fn new() -> Self {
+ let int64_coercion = Coercion::new_implicit(
+
datafusion::logical_expr::TypeSignatureClass::Native(logical_int64()),
+ vec![],
+ NativeType::Int64,
+ );
+
+ Self {
+ signature: Signature::one_of(
+ vec![
+ // Support 1 to 7 parameters
Review Comment:
I feel we can reduce the number of parameters here, and for the invoke do
something as simple as outputting a string of which parameters were provided,
to make it more clear which parameters were actually successfully passed
through.
--
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]
