alamb commented on code in PR #6662:
URL: https://github.com/apache/arrow-datafusion/pull/6662#discussion_r1244313363
##########
datafusion/expr/src/built_in_function.rs:
##########
@@ -440,46 +440,44 @@ impl BuiltinScalarFunction {
// the return type of the built in function.
// Some built-in functions' return type depends on the incoming type.
match self {
- BuiltinScalarFunction::ArrayAppend => match &input_expr_types[0] {
- List(field) => Ok(List(Arc::new(Field::new(
- "item",
- field.data_type().clone(),
- true,
- )))),
- _ => Err(DataFusionError::Internal(format!(
- "The {self} function can only accept list as the first
argument"
- ))),
- },
- BuiltinScalarFunction::ArrayConcat => match &input_expr_types[0] {
- List(field) => Ok(List(Arc::new(Field::new(
- "item",
- field.data_type().clone(),
- true,
- )))),
- _ => Err(DataFusionError::Internal(format!(
- "The {self} function can only accept fixed size list as
the args."
- ))),
- },
- BuiltinScalarFunction::ArrayDims => Ok(UInt8),
- BuiltinScalarFunction::ArrayFill => Ok(List(Arc::new(Field::new(
+ BuiltinScalarFunction::ArrayAppend => Ok(List(Arc::new(Field::new(
"item",
- input_expr_types[0].clone(),
+ input_expr_types[1].clone(),
true,
)))),
+ BuiltinScalarFunction::ArrayConcat => {
+ let mut expr_type = Null;
+ for input_expr_type in input_expr_types {
+ match input_expr_type {
+ List(field) => {
+ if !field.data_type().equals_datatype(&Null) {
+ expr_type = field.data_type().clone();
+ break;
+ }
+ }
+ _ => {
+ return Err(DataFusionError::Internal(format!(
Review Comment:
Indeed: #6108
##########
datafusion/physical-expr/src/array_expressions.rs:
##########
@@ -56,20 +67,29 @@ macro_rules! new_builder {
macro_rules! array {
($ARGS:expr, $ARRAY_TYPE:ident, $BUILDER_TYPE:ident) => {{
- // downcast all arguments to their common format
- let args =
- downcast_vec!($ARGS,
$ARRAY_TYPE).collect::<Result<Vec<&$ARRAY_TYPE>>>()?;
-
- let builder = new_builder!($BUILDER_TYPE, args[0].len());
+ let builder = new_builder!($BUILDER_TYPE, $ARGS[0].len());
let mut builder =
- ListBuilder::<$BUILDER_TYPE>::with_capacity(builder, args.len());
+ ListBuilder::<$BUILDER_TYPE>::with_capacity(builder, $ARGS.len());
+
// for each entry in the array
- for index in 0..args[0].len() {
- for arg in &args {
- if arg.is_null(index) {
- builder.values().append_null();
- } else {
- builder.values().append_value(arg.value(index));
+ for index in 0..$ARGS[0].len() {
Review Comment:
I would expect that the array contains nulls values
Like
```
select make_array(1, Null, 2 Null);
----
1, NULL, 2, NULL
```
Where the element of the ListArray are marked as null 🤔
--
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]
