Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: array-empty #7313

Merged
merged 14 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion datafusion/expr/src/built_in_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ pub enum BuiltinScalarFunction {
ArrayDims,
/// array_element
ArrayElement,
/// array_empty
ArrayEmpty,
/// array_length
ArrayLength,
/// array_ndims
Expand Down Expand Up @@ -360,6 +362,7 @@ impl BuiltinScalarFunction {
BuiltinScalarFunction::Trunc => Volatility::Immutable,
BuiltinScalarFunction::ArrayAppend => Volatility::Immutable,
BuiltinScalarFunction::ArrayConcat => Volatility::Immutable,
BuiltinScalarFunction::ArrayEmpty => Volatility::Immutable,
BuiltinScalarFunction::ArrayHasAll => Volatility::Immutable,
BuiltinScalarFunction::ArrayHasAny => Volatility::Immutable,
BuiltinScalarFunction::ArrayHas => Volatility::Immutable,
Expand Down Expand Up @@ -538,7 +541,8 @@ impl BuiltinScalarFunction {
}
BuiltinScalarFunction::ArrayHasAll
| BuiltinScalarFunction::ArrayHasAny
| BuiltinScalarFunction::ArrayHas => Ok(Boolean),
| BuiltinScalarFunction::ArrayHas
| BuiltinScalarFunction::ArrayEmpty => Ok(Boolean),
BuiltinScalarFunction::ArrayDims => {
Ok(List(Arc::new(Field::new("item", UInt64, true))))
}
Expand Down Expand Up @@ -832,6 +836,7 @@ impl BuiltinScalarFunction {
Signature::variadic_any(self.volatility())
}
BuiltinScalarFunction::ArrayDims => Signature::any(1, self.volatility()),
BuiltinScalarFunction::ArrayEmpty => Signature::any(1, self.volatility()),
BuiltinScalarFunction::ArrayElement => Signature::any(2, self.volatility()),
BuiltinScalarFunction::Flatten => Signature::any(1, self.volatility()),
BuiltinScalarFunction::ArrayHasAll
Expand Down Expand Up @@ -1322,6 +1327,7 @@ fn aliases(func: &BuiltinScalarFunction) -> &'static [&'static str] {
&["array_concat", "array_cat", "list_concat", "list_cat"]
}
BuiltinScalarFunction::ArrayDims => &["array_dims", "list_dims"],
BuiltinScalarFunction::ArrayEmpty => &["empty"],
BuiltinScalarFunction::ArrayElement => &[
"array_element",
"array_extract",
Expand Down
6 changes: 6 additions & 0 deletions datafusion/expr/src/expr_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,12 @@ scalar_expr!(
first_array second_array,
"Returns true, if the element appears in the first array, otherwise false."
);
scalar_expr!(
ArrayEmpty,
array_empty,
array,
"returns 1 for an empty array or 0 for a non-empty array."
);
scalar_expr!(
ArrayHasAll,
array_has_all,
Expand Down
21 changes: 21 additions & 0 deletions datafusion/physical-expr/src/array_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,27 @@ macro_rules! general_repeat_list {
}};
}

/// Array_empty SQL function
pub fn array_empty(args: &[ArrayRef]) -> Result<ArrayRef> {
alamb marked this conversation as resolved.
Show resolved Hide resolved
println!("args[0]: {:?}", &args[0]);
if args[0].as_any().downcast_ref::<NullArray>().is_some() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this doesn't catch all nulls. This only works within the test setup because array_empty(NULL) creates a NULL array. I think this will fail w/:

SELECT array_empty(a)
FROM VALUES(NULL, make_array(), make_array(1)) sub (a);

I suggest you remove this check and change the else part within array.iter().map(...) to None (from Some(true)).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole handling of nulls in the array function logic is "maturing" I would say (e.g. #7142 and others listed on #6980)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but that doesn't mean we have to add more immature code, esp. when the fix is rather simple (I would argue the fixed code is even simpler than the current version).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest you remove this check and change the else part within array.iter().map(...) to None (from Some(true)).

It cannot handle all NULL array cases in this way, but I think we can wait #7142 to make this pr better.

return Ok(args[0].clone());
}

let array = as_list_array(&args[0])?;
let builder = array
.iter()
.map(|arr| {
if let Some(arr) = arr {
Some(arr.len() == arr.null_count())
} else {
None
}
})
.collect::<BooleanArray>();
Ok(Arc::new(builder))
}

/// Array_repeat SQL function
pub fn array_repeat(args: &[ArrayRef]) -> Result<ArrayRef> {
let element = &args[0];
Expand Down
3 changes: 3 additions & 0 deletions datafusion/physical-expr/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,9 @@ pub fn create_physical_fun(
BuiltinScalarFunction::ArrayConcat => {
Arc::new(|args| make_scalar_function(array_expressions::array_concat)(args))
}
BuiltinScalarFunction::ArrayEmpty => {
Arc::new(|args| make_scalar_function(array_expressions::array_empty)(args))
}
BuiltinScalarFunction::ArrayHasAll => {
Arc::new(|args| make_scalar_function(array_expressions::array_has_all)(args))
}
Expand Down
1 change: 1 addition & 0 deletions datafusion/proto/proto/datafusion.proto
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ enum ScalarFunction {
Flatten = 112;
Isnan = 113;
Iszero = 114;
ArrayEmpty = 115;
}

message ScalarFunctionNode {
Expand Down
3 changes: 3 additions & 0 deletions datafusion/proto/src/generated/pbjson.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions datafusion/proto/src/generated/prost.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion datafusion/proto/src/logical_plan/from_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use datafusion_common::{
Column, DFField, DFSchema, DFSchemaRef, DataFusionError, OwnedTableReference, Result,
ScalarValue,
};
use datafusion_expr::expr::{Alias, Placeholder};
use datafusion_expr::{
abs, acos, acosh, array, array_append, array_concat, array_dims, array_element,
array_has, array_has_all, array_has_any, array_length, array_ndims, array_position,
Expand All @@ -59,6 +58,10 @@ use datafusion_expr::{
JoinConstraint, JoinType, Like, Operator, TryCast, WindowFrame, WindowFrameBound,
WindowFrameUnits,
};
use datafusion_expr::{
array_empty,
expr::{Alias, Placeholder},
};
use std::sync::Arc;

#[derive(Debug)]
Expand Down Expand Up @@ -452,6 +455,7 @@ impl From<&protobuf::ScalarFunction> for BuiltinScalarFunction {
ScalarFunction::ToTimestamp => Self::ToTimestamp,
ScalarFunction::ArrayAppend => Self::ArrayAppend,
ScalarFunction::ArrayConcat => Self::ArrayConcat,
ScalarFunction::ArrayEmpty => Self::ArrayEmpty,
alamb marked this conversation as resolved.
Show resolved Hide resolved
ScalarFunction::ArrayHasAll => Self::ArrayHasAll,
ScalarFunction::ArrayHasAny => Self::ArrayHasAny,
ScalarFunction::ArrayHas => Self::ArrayHas,
Expand Down Expand Up @@ -1355,6 +1359,9 @@ pub fn parse_expr(
parse_expr(&args[0], registry)?,
parse_expr(&args[1], registry)?,
)),
ScalarFunction::ArrayEmpty => {
Ok(array_empty(parse_expr(&args[0], registry)?))
}
ScalarFunction::ArrayNdims => {
Ok(array_ndims(parse_expr(&args[0], registry)?))
}
Expand Down
1 change: 1 addition & 0 deletions datafusion/proto/src/logical_plan/to_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1451,6 +1451,7 @@ impl TryFrom<&BuiltinScalarFunction> for protobuf::ScalarFunction {
BuiltinScalarFunction::ToTimestamp => Self::ToTimestamp,
BuiltinScalarFunction::ArrayAppend => Self::ArrayAppend,
BuiltinScalarFunction::ArrayConcat => Self::ArrayConcat,
BuiltinScalarFunction::ArrayEmpty => Self::ArrayEmpty,
BuiltinScalarFunction::ArrayHasAll => Self::ArrayHasAll,
BuiltinScalarFunction::ArrayHasAny => Self::ArrayHasAny,
BuiltinScalarFunction::ArrayHas => Self::ArrayHas,
Expand Down
36 changes: 36 additions & 0 deletions datafusion/sqllogictest/test_files/array.slt
Original file line number Diff line number Diff line change
Expand Up @@ -2363,6 +2363,42 @@ from flatten_table;
[1, 2, 3] [1, 2, 3, 4, 5, 6] [1, 2, 3] [1.0, 2.1, 2.2, 3.2, 3.3, 3.4]
[1, 2, 3, 4, 5, 6] [8] [1, 2, 3] [1.0, 2.0, 3.0, 4.0, 5.0, 6.0]

# empty scalar function #1
query B
select empty(make_array(1));
----
false

# empty scalar function #2
query B
select empty(make_array());
----
true

# empty scalar function #3
query B
select empty(make_array(NULL));
----
true

# empty scalar function #4
query B
select empty(NULL);
----
NULL

# empty scalar function #5
query B
select empty(column1) from arrays;
----
false
false
false
false
NULL
false
false

alamb marked this conversation as resolved.
Show resolved Hide resolved
### Delete tables

statement ok
Expand Down
27 changes: 27 additions & 0 deletions docs/source/user-guide/sql/scalar_functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -1495,6 +1495,7 @@ from_unixtime(expression)
- [array_slice](#array_slice)
- [array_to_string](#array_to_string)
- [cardinality](#cardinality)
- [empty](#empty)
- [list_append](#list_append)
- [list_cat](#list_cat)
- [list_concat](#list_concat)
Expand Down Expand Up @@ -1693,6 +1694,8 @@ array_element(array, index)
- list_element
- list_extract

### `array_empty`

### `array_extract`

_Alias of [array_element](#array_element)._
Expand Down Expand Up @@ -2188,6 +2191,30 @@ cardinality(array)
+--------------------------------------+
```

### `empty`

Returns 1 for an empty array or 0 for a non-empty array.

```
empty(array)
```

#### Arguments

- **array**: Array expression.
Can be a constant, column, or function, and any combination of array operators.

#### Example

```
❯ select empty([1]);
+------------------+
| empty(List([1])) |
+------------------+
| 0 |
+------------------+
```

### `list_append`

_Alias of [array_append](#array_append)._
Expand Down
Loading