ologlogn commented on code in PR #21895:
URL: https://github.com/apache/datafusion/pull/21895#discussion_r3274562278
##########
datafusion/sqllogictest/test_files/array/array_transform.slt:
##########
@@ -423,6 +421,169 @@ SELECT array_transform([1], v -> v -> v+1);
query error DataFusion error: SQL error: ParserError\("Expected: an
expression, found: \) at Line: 1, Column: 30"\)
SELECT array_transform([1], () -> 1);
+##############
+## array_filter tests
+##############
+
+query ?
+SELECT array_filter([1, 2, 3, 4, 5], v -> v > 2);
+----
+[3, 4, 5]
+
+query ?
+SELECT list_filter([1, 2, 3, 4, 5], v -> v > 2);
+----
+[3, 4, 5]
+
+# multiple sublists — t.list rows are [1,50], [4,50], [7,50]
+query ?
+SELECT array_filter(list, v -> v > 40) from t;
+----
+[50]
+[50]
+[50]
+
+# filter with column capture in predicate
+query ?
+SELECT array_filter(list, v -> v > t.number) from t;
+----
+[50]
+[50]
+[]
+
+# null sublist is preserved
+query ?
+SELECT array_filter(list, v -> v > 1) from with_null_list;
+----
+[2]
+NULL
+
+# all null fast path
+query ?
+SELECT array_filter(list, v -> v > 1) from fully_null_list;
+----
+NULL
+NULL
+
+# empty sublists fast path
+query ?
+SELECT array_filter([], v -> v > 1);
+----
+[]
+
+# scalar true: return list unchanged
+query ?
+SELECT array_filter([1, 2, 3], v -> true);
+----
+[1, 2, 3]
+
+# scalar false: return empty sublists
+query ?
+SELECT array_filter([1, 2, 3], v -> false);
+----
+[]
+
+# all filtered out
+query ?
+SELECT array_filter([1, 2], v -> v > 10);
+----
+[]
+
+# nothing filtered — all elements pass predicate
+query ?
+SELECT array_filter([3, 4, 5], v -> v > 2);
+----
+[3, 4, 5]
+
+# coercion: ListView input
+query ?
+SELECT array_filter(arrow_cast(list, 'ListView(Int32)'), v -> v > 2) from t;
+----
+[50]
+[4, 50]
+[7, 50]
+
+# null array argument returns null
+query ?
+SELECT array_filter(arrow_cast(NULL, 'List(Int32)'), v -> v > 0);
+----
+NULL
+
+# lambda returns null for some elements — null treated as false (element
dropped)
+query ?
+SELECT array_filter([1, 2, 3], v -> CASE WHEN v = 2 THEN NULL ELSE v > 1 END);
+----
+[3]
+
+# lambda always returns null — scalar null predicate short-circuit returns
empty list
+query ?
+SELECT array_filter([1, 2, 3], v -> CAST(NULL AS BOOLEAN));
+----
+[]
+
+query error DataFusion error: Error during planning: array_filter requires 1
value argument, got 0
+SELECT array_filter();
+
+query error DataFusion error: Error during planning: array_filter expected a
list as first argument, got Int64
+SELECT array_filter(1, v -> v > 0);
+
Review Comment:
This test already exists in `array_filter.slt` — `SELECT array_filter(v -> v
> 0, [1, 2, 3]);` with the updated error message from the `Exact` signature.
##########
datafusion/functions-nested/src/array_filter.rs:
##########
@@ -0,0 +1,486 @@
+// 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.
+
+//! [`HigherOrderUDF`] definitions for array_filter function.
+
+use arrow::{
+ array::{
+ Array, ArrayRef, AsArray, BooleanArray, BooleanBufferBuilder,
LargeListArray,
+ ListArray, OffsetSizeTrait, new_empty_array,
+ },
+ buffer::{OffsetBuffer, ScalarBuffer},
+ compute::filter as arrow_filter,
+ datatypes::{DataType, Field, FieldRef},
+};
+use datafusion_common::{
+ Result, ScalarValue, exec_err, plan_err, utils::adjust_offsets_for_slice,
+};
+use datafusion_expr::{
+ ColumnarValue, Documentation, HigherOrderFunctionArgs,
HigherOrderReturnFieldArgs,
+ HigherOrderSignature, HigherOrderUDF, LambdaParametersProgress,
ValueOrLambda,
+ Volatility,
+};
+use datafusion_macros::user_doc;
+use std::sync::Arc;
+
+make_higher_order_function_expr_and_func!(
+ ArrayFilter,
+ array_filter,
+ array lambda,
+ "filters the values of an array using a boolean lambda",
+ array_filter_higher_order_function
+);
+
+#[user_doc(
+ doc_section(label = "Array Functions"),
+ description = "filters the values of an array using a boolean lambda",
+ syntax_example = "array_filter(array, x -> x > 2)",
+ sql_example = r#"```sql
+> select array_filter([1, 2, 3, 4, 5], x -> x > 2);
++--------------------------------------------+
+| array_filter([1, 2, 3, 4, 5], x -> x > 2) |
++--------------------------------------------+
+| [3, 4, 5] |
++--------------------------------------------+
+```"#,
+ argument(
+ name = "array",
+ description = "Array expression. Can be a constant, column, or
function, and any combination of array operators."
+ ),
+ argument(
+ name = "lambda",
+ description = "Lambda that returns a boolean. Elements for which the
lambda returns true are kept."
+ )
+)]
+#[derive(Debug, PartialEq, Eq, Hash)]
+pub struct ArrayFilter {
+ signature: HigherOrderSignature,
+ aliases: Vec<String>,
+}
+
+impl Default for ArrayFilter {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+impl ArrayFilter {
+ pub fn new() -> Self {
+ Self {
+ signature:
HigherOrderSignature::user_defined(Volatility::Immutable),
Review Comment:
Done — rebased from main and switched to `HigherOrderSignature::exact`.
##########
datafusion/functions-nested/src/array_transform.rs:
##########
@@ -100,53 +98,15 @@ impl HigherOrderUDF for ArrayTransform {
}
fn coerce_value_types(&self, arg_types: &[DataType]) ->
Result<Vec<DataType>> {
- let [list] = arg_types else {
- return plan_err!(
- "{} function requires 1 value argument, got {}",
- self.name(),
- arg_types.len()
- );
- };
-
- let coerced = match list {
- DataType::List(_) | DataType::LargeList(_) => list.clone(),
- DataType::ListView(field) | DataType::FixedSizeList(field, _) => {
- DataType::List(Arc::clone(field))
- }
- DataType::LargeListView(field) =>
DataType::LargeList(Arc::clone(field)),
- _ => {
- return plan_err!(
- "{} expected a list as first argument, got {}",
- self.name(),
- list
- );
- }
- };
-
- Ok(vec![coerced])
+ crate::lambda_utils::coerce_single_list_arg(self.name(), arg_types)
Review Comment:
Fixed — added proper `use` statements at the top.
##########
datafusion/functions-nested/src/array_transform.rs:
##########
@@ -187,31 +147,13 @@ impl HigherOrderUDF for ArrayTransform {
let list_array = list.to_array(args.number_rows)?;
- // Fast path for fully null input array
- if list_array.null_count() == list_array.len() {
- return Ok(ColumnarValue::Scalar(ScalarValue::try_new_null(
- args.return_type(),
- )?));
- }
-
- // as per list_values docs, if list_array is sliced, list_values will
be sliced too,
- // so before constructing the transformed array below, we must adjust
the list offsets with
- // adjust_offsets_for_slice
- let list_values = list_values(&list_array)?;
-
- // fast path: when every sublist is empty and non-null we can return a
scalar of an non-null empty sublist.
- // If every sublist is null have already been handled above
- if list_values.is_empty()
- && list_array.null_count() == 0
- && matches!(
- args.return_type(),
- DataType::List(_) | DataType::LargeList(_)
- )
- {
- return Ok(ColumnarValue::Scalar(ScalarValue::new_default(
- args.return_type(),
- )?));
- }
+ let list_values = match crate::lambda_utils::extract_list_values(
+ &list_array,
+ args.return_type(),
+ )? {
+ crate::lambda_utils::ListValuesResult::EarlyReturn(v) => return
Ok(v),
+ crate::lambda_utils::ListValuesResult::Values(v) => v,
+ };
Review Comment:
Fixed.
##########
datafusion/functions-nested/src/test_utils.rs:
##########
@@ -0,0 +1,89 @@
+// 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.
+
+//! Test helpers shared across higher-order function tests.
+
+use std::{collections::HashMap, sync::Arc};
+
+use arrow::{
+ array::{Array, ArrayRef, Int32Array, ListArray, RecordBatch},
+ buffer::{NullBuffer, OffsetBuffer},
+ datatypes::{DataType, Field},
+};
+use datafusion_common::{DFSchema, Result};
+use datafusion_expr::{
+ Expr, HigherOrderUDF, col,
+ execution_props::ExecutionProps,
+ expr::{HigherOrderFunction, LambdaVariable},
+ lambda,
+};
+use datafusion_physical_expr::create_physical_expr;
+
+/// Creates a `ListArray` of `Int32` values with the given flat values,
sublist offsets, and nulls.
+pub(crate) fn create_i32_list(
Review Comment:
Done — moved to `lambda_utils.rs` under `#[cfg(test)] pub(crate) mod
test_utils`, deleted `test_utils.rs`.
--
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]
