martin-g commented on code in PR #18450:
URL: https://github.com/apache/datafusion/pull/18450#discussion_r2498962715
##########
datafusion/core/tests/user_defined/user_defined_scalar_functions.rs:
##########
@@ -1112,6 +1140,127 @@ async fn create_scalar_function_from_sql_statement() ->
Result<()> {
Ok(())
}
+#[tokio::test]
+async fn create_scalar_function_from_sql_statement_named_arguments() ->
Result<()> {
+ let function_factory = Arc::new(CustomFunctionFactory::default());
+ let ctx =
SessionContext::new().with_function_factory(function_factory.clone());
+
+ let sql = r#"
+ CREATE FUNCTION better_add(a DOUBLE, b DOUBLE)
+ RETURNS DOUBLE
+ RETURN $a + $b
+ "#;
+
+ assert!(ctx.sql(sql).await.is_ok());
+
+ let result = ctx
+ .sql("select better_add(2.0, 2.0)")
+ .await?
+ .collect()
+ .await?;
+
+ assert_batches_eq!(
+ &[
+ "+-----------------------------------+",
+ "| better_add(Float64(2),Float64(2)) |",
+ "+-----------------------------------+",
+ "| 4.0 |",
+ "+-----------------------------------+",
+ ],
+ &result
+ );
+
+ // cannot mix named and positional style
+ let bad_expression_sql = r#"
+ CREATE FUNCTION bad_expression_fun(DOUBLE, b DOUBLE)
+ RETURNS DOUBLE
+ RETURN $1 + $b
+ "#;
+ let err = ctx
+ .sql(bad_expression_sql)
+ .await
+ .expect_err("cannot mix named and positional style");
+ let expected = "Error during planning: All function arguments must use
either named or positional style.";
+ assert!(expected.starts_with(&err.strip_backtrace()));
+
+ Ok(())
+}
+
+#[tokio::test]
+async fn create_scalar_function_from_sql_statement_default_arguments() ->
Result<()> {
+ let function_factory = Arc::new(CustomFunctionFactory::default());
+ let ctx =
SessionContext::new().with_function_factory(function_factory.clone());
+
+ let sql = r#"
+ CREATE FUNCTION better_add(a DOUBLE DEFAULT 2.0, b DOUBLE DEFAULT 2.0)
+ RETURNS DOUBLE
+ RETURN $a + $b
+ "#;
+
+ assert!(ctx.sql(sql).await.is_ok());
+
+ // Check all function arity supported
+ let result = ctx.sql("select better_add()").await?.collect().await?;
+
+ assert_batches_eq!(
+ &[
+ "+--------------+",
+ "| better_add() |",
+ "+--------------+",
+ "| 4.0 |",
+ "+--------------+",
+ ],
+ &result
+ );
+
+ let result = ctx.sql("select better_add(2.0)").await?.collect().await?;
+
+ assert_batches_eq!(
+ &[
+ "+------------------------+",
+ "| better_add(Float64(2)) |",
+ "+------------------------+",
+ "| 4.0 |",
+ "+------------------------+",
+ ],
+ &result
+ );
+
+ let result = ctx
+ .sql("select better_add(2.0, 2.0)")
+ .await?
+ .collect()
+ .await?;
+
+ assert_batches_eq!(
+ &[
+ "+-----------------------------------+",
+ "| better_add(Float64(2),Float64(2)) |",
+ "+-----------------------------------+",
+ "| 4.0 |",
+ "+-----------------------------------+",
+ ],
+ &result
+ );
+
+ assert!(ctx.sql("select better_add(2.0, 2.0, 2.0)").await.is_err());
+
+ // non-default argument cannot follow default argument
+ let bad_expression_sql = r#"
+ CREATE FUNCTION bad_expression_fun(a DOUBLE DEFAULT 2.0, b DOUBLE)
+ RETURNS DOUBLE
+ RETURN $a + $b
+ "#;
+ let err = ctx
+ .sql(bad_expression_sql)
+ .await
+ .expect_err("non-default argument cannot follow default argument");
+ let expected =
+ "Error during planning: Non-default arguments cannot follow default
arguments.";
+ assert!(expected.starts_with(&err.strip_backtrace()));
+ Ok(())
+}
+
Review Comment:
Please add a test case with positional parameter that is not existing, e.g.
`$5` where there are less than 5 arguments.
I have the feeling that it will fail with `index out of bounds` error at
https://github.com/apache/datafusion/pull/18450/files#diff-647d2e08b4d044bf63b35f9e23092ba9673b80b1568e8f3abffd7f909552ea1aR999
You need to add a check similar to `if placeholder_position < defaults.len()
{...}` around it and return an error in the `else` clause
--
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]
