alamb commented on code in PR #6601:
URL: https://github.com/apache/arrow-datafusion/pull/6601#discussion_r1223089700


##########
datafusion/sql/tests/integration_test.rs:
##########
@@ -125,12 +127,78 @@ fn parse_ident_normalization() {
             ParserOptions {
                 parse_float_as_decimal: false,
                 enable_ident_normalization,
+                prioritize_udf: false,
             },
         );
         assert_eq!(expected, format!("{plan:?}"));
     }
 }
 
+#[test]
+fn parse_prioritize_udf() {
+    // Create a UDF that has the same name as a builtin function (abs)
+    let return_type: ReturnTypeFunction =
+        Arc::new(move |_| Ok(Arc::new(DataType::Int64)));
+    let fun: ScalarFunctionImplementation =
+        Arc::new(move |_| 
Ok(ColumnarValue::Scalar(ScalarValue::Int64(Some(1)))));
+    let scalar_udf = Arc::new(ScalarUDF::new(
+        "abs",
+        &Signature::uniform(1, vec![DataType::Float32], Volatility::Stable),
+        &return_type,
+        &fun,
+    ));
+
+    // Test data
+    let test_data = [

Review Comment:
   Rather than checking the plan here, what do you think about simply running 
the query and verifying the output?
   
   For example, you could run the query
   
   ```
   SELECT ABS(-100)
   ```
   
   And if it returns `1` you know the UDF is called, and if it returns `100` 
you know the built in was called



##########
datafusion/common/src/config.rs:
##########
@@ -191,6 +191,8 @@ config_namespace! {
         /// MySQL, PostgreSQL, Hive, SQLite, Snowflake, Redshift, MsSQL, 
ClickHouse, BigQuery, and Ansi.
         pub dialect: String, default = "generic".to_string()
 
+        /// Should DataFusion prioritize UDF over built-in scalar function in 
case of function name conflict.

Review Comment:
   I actually don't think a config flag is needed -- if a UDF is registered 
with the same name as a built in, it will not be callable (as you note in the 
issue / PR description). Therefore I think it is safe to assume that the if a 
user registers a function with the same name as a built in they want the new 
function to have precidence



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to