Jefffrey commented on code in PR #21542:
URL: https://github.com/apache/datafusion/pull/21542#discussion_r3104880083


##########
datafusion/functions-nested/src/cosine_distance.rs:
##########
@@ -0,0 +1,202 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! [`ScalarUDFImpl`] definitions for cosine_distance function.
+
+use crate::utils::make_scalar_function;
+use arrow::array::{Array, ArrayRef, Float64Array, OffsetSizeTrait};
+use arrow::datatypes::{
+    DataType,
+    DataType::{FixedSizeList, LargeList, List, Null},
+};
+use datafusion_common::cast::{as_float64_array, as_generic_list_array};
+use datafusion_common::utils::{ListCoercion, coerced_type_with_base_type_only};
+use datafusion_common::{Result, exec_err, plan_err, utils::take_function_args};
+use datafusion_expr::{
+    ColumnarValue, Documentation, ScalarFunctionArgs, ScalarUDFImpl, Signature,
+    Volatility,
+};
+use datafusion_macros::user_doc;
+use itertools::Itertools;
+use std::sync::Arc;
+
+make_udf_expr_and_func!(
+    CosineDistance,
+    cosine_distance,
+    array1 array2,
+    "returns the cosine distance between two numeric arrays.",
+    cosine_distance_udf
+);
+
+#[user_doc(
+    doc_section(label = "Array Functions"),
+    description = "Returns the cosine distance between two input arrays of 
equal length. The cosine distance is defined as 1 - cosine_similarity, i.e. `1 
- dot(a,b) / (||a|| * ||b||)`. Returns NULL if either array is NULL or contains 
only zeros.",
+    syntax_example = "cosine_distance(array1, array2)",
+    sql_example = r#"```sql
+> select cosine_distance([1.0, 0.0], [0.0, 1.0]);
++-----------------------------------------------+
+| cosine_distance(List([1.0,0.0]),List([0.0,1.0])) |
++-----------------------------------------------+
+| 1.0                                           |
++-----------------------------------------------+
+```"#,
+    argument(
+        name = "array1",
+        description = "Array expression. Can be a constant, column, or 
function, and any combination of array operators."
+    ),
+    argument(
+        name = "array2",
+        description = "Array expression. Can be a constant, column, or 
function, and any combination of array operators."
+    )
+)]
+#[derive(Debug, PartialEq, Eq, Hash)]
+pub struct CosineDistance {
+    signature: Signature,
+    aliases: Vec<String>,
+}
+
+impl Default for CosineDistance {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl CosineDistance {
+    pub fn new() -> Self {
+        Self {
+            signature: Signature::user_defined(Volatility::Immutable),
+            aliases: vec!["list_cosine_distance".to_string()],
+        }
+    }
+}
+
+impl ScalarUDFImpl for CosineDistance {
+    fn name(&self) -> &str {
+        "cosine_distance"
+    }
+
+    fn signature(&self) -> &Signature {
+        &self.signature
+    }
+
+    fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
+        Ok(DataType::Float64)
+    }
+
+    fn coerce_types(&self, arg_types: &[DataType]) -> Result<Vec<DataType>> {

Review Comment:
   Something to keep in mind here is input lists of different types, take for 
example:
   
   ```sql
   query R
   select list_cosine_distance([1.0, 0.0], arrow_cast([0.0, 1.0], 
'LargeList(Float16)'));
   ----
   1
   ```
   
   This was highlighted by the recent PR for concat:
   
   - https://github.com/apache/datafusion/pull/21704



##########
datafusion/sqllogictest/test_files/cosine_distance.slt:
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## cosine_distance
+
+# Orthogonal vectors: distance = 1.0
+query R
+select cosine_distance([1.0, 0.0], [0.0, 1.0]);
+----
+1
+
+# Identical vectors: distance = 0.0
+query R
+select cosine_distance([1.0, 2.0, 3.0], [1.0, 2.0, 3.0]);
+----
+0
+
+# Opposite vectors: distance = 2.0
+query R
+select cosine_distance([1.0, 0.0], [-1.0, 0.0]);
+----
+2
+
+# 45-degree angle: distance ≈ 0.293
+query R
+select round(cosine_distance([1.0, 0.0], [1.0, 1.0]), 3);
+----
+0.293
+
+# NULL input (bare NULL is not a list type, errors at planning)
+query error cosine_distance does not support type
+select cosine_distance(NULL, [1.0, 2.0]);
+
+# NULL in second position
+query error cosine_distance does not support type
+select cosine_distance([1.0, 2.0], NULL);
+
+# Zero vector returns NULL (undefined cosine similarity)
+query R
+select cosine_distance([0.0, 0.0], [1.0, 2.0]);
+----
+NULL
+
+# Mismatched lengths error
+query error cosine_distance requires both list inputs to have the same length
+select cosine_distance([1.0, 2.0], [1.0]);
+
+# NULL element inside a list returns NULL for that row
+query R
+select cosine_distance([1.0, 2.0, NULL], [1.0, 2.0, 3.0]);
+----
+NULL
+
+# LargeList support
+query R
+select cosine_distance(
+    arrow_cast([1.0, 0.0], 'LargeList(Float64)'),
+    arrow_cast([0.0, 1.0], 'LargeList(Float64)')
+);
+----
+1
+
+# Integer arrays (coerced to Float64)
+query R
+select cosine_distance([1, 0], [0, 1]);
+----
+1
+
+# Multi-row query
+query R
+select cosine_distance(column1, column2) from (values
+    (make_array(1.0, 0.0), make_array(0.0, 1.0)),
+    (make_array(1.0, 1.0), make_array(1.0, 1.0)),
+    (make_array(1.0, 0.0), make_array(-1.0, 0.0))

Review Comment:
   ```suggestion
       (make_array(1.0, 0.0), make_array(-1.0, 0.0)),
       (make_array(1.0, 0.0), NULL)
   ```
   
   Just for coverage



##########
datafusion/sqllogictest/test_files/cosine_distance.slt:
##########
@@ -0,0 +1,115 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## cosine_distance
+
+# Orthogonal vectors: distance = 1.0
+query R
+select cosine_distance([1.0, 0.0], [0.0, 1.0]);
+----
+1
+
+# Identical vectors: distance = 0.0
+query R
+select cosine_distance([1.0, 2.0, 3.0], [1.0, 2.0, 3.0]);
+----
+0
+
+# Opposite vectors: distance = 2.0
+query R
+select cosine_distance([1.0, 0.0], [-1.0, 0.0]);
+----
+2
+
+# 45-degree angle: distance ≈ 0.293
+query R
+select round(cosine_distance([1.0, 0.0], [1.0, 1.0]), 3);
+----
+0.293
+
+# NULL input (bare NULL is not a list type, errors at planning)
+query error cosine_distance does not support type
+select cosine_distance(NULL, [1.0, 2.0]);

Review Comment:
   We could probably still handle this to return null



##########
datafusion/functions-nested/src/cosine_distance.rs:
##########
@@ -0,0 +1,202 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! [`ScalarUDFImpl`] definitions for cosine_distance function.
+
+use crate::utils::make_scalar_function;
+use arrow::array::{Array, ArrayRef, Float64Array, OffsetSizeTrait};
+use arrow::datatypes::{
+    DataType,
+    DataType::{FixedSizeList, LargeList, List, Null},
+};
+use datafusion_common::cast::{as_float64_array, as_generic_list_array};
+use datafusion_common::utils::{ListCoercion, coerced_type_with_base_type_only};
+use datafusion_common::{Result, exec_err, plan_err, utils::take_function_args};
+use datafusion_expr::{
+    ColumnarValue, Documentation, ScalarFunctionArgs, ScalarUDFImpl, Signature,
+    Volatility,
+};
+use datafusion_macros::user_doc;
+use itertools::Itertools;
+use std::sync::Arc;
+
+make_udf_expr_and_func!(
+    CosineDistance,
+    cosine_distance,
+    array1 array2,
+    "returns the cosine distance between two numeric arrays.",
+    cosine_distance_udf
+);
+
+#[user_doc(
+    doc_section(label = "Array Functions"),
+    description = "Returns the cosine distance between two input arrays of 
equal length. The cosine distance is defined as 1 - cosine_similarity, i.e. `1 
- dot(a,b) / (||a|| * ||b||)`. Returns NULL if either array is NULL or contains 
only zeros.",
+    syntax_example = "cosine_distance(array1, array2)",
+    sql_example = r#"```sql
+> select cosine_distance([1.0, 0.0], [0.0, 1.0]);
++-----------------------------------------------+
+| cosine_distance(List([1.0,0.0]),List([0.0,1.0])) |
++-----------------------------------------------+
+| 1.0                                           |
++-----------------------------------------------+
+```"#,
+    argument(
+        name = "array1",
+        description = "Array expression. Can be a constant, column, or 
function, and any combination of array operators."
+    ),
+    argument(
+        name = "array2",
+        description = "Array expression. Can be a constant, column, or 
function, and any combination of array operators."
+    )
+)]
+#[derive(Debug, PartialEq, Eq, Hash)]
+pub struct CosineDistance {
+    signature: Signature,
+    aliases: Vec<String>,
+}
+
+impl Default for CosineDistance {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl CosineDistance {
+    pub fn new() -> Self {
+        Self {
+            signature: Signature::user_defined(Volatility::Immutable),
+            aliases: vec!["list_cosine_distance".to_string()],

Review Comment:
   `list_cosine_distance` seems a bit unwieldy, especially considering the 
regular name of the function `cosine_distance` doesn't mention array (usually 
we'd have something like `array_remove` then the alias just replaces `array`, 
e.g. `list_remove`)



-- 
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]

Reply via email to