`select array_concat([])` panicked

[jonahgao]

Describe the bug

thread 'main' panicked at datafusion/datafusion/functions-array/src/concat.rs:275:32:
index out of bounds: the len is 0 but the index is 0

To Reproduce

Run any of the following queries in CLI.

DataFusion CLI v37.1.0
> select array_concat([]);

DataFusion CLI v37.1.0
>  select array_concat(make_array());

Expected behavior

No response

Additional context

No response

[Lordworms]

I can fix this one if no one is working on this

[jayzhan211]

@Lordworms Not sure what is your idea to solve this problem. I expect this to be solved with the correct signature.
I had worked on #8594 before but the solution is quite a specialized case. If you come out with another better idea, it would be great!

[Lordworms]

select array_concat(make_array());

Hi @jayzhan211 I was going to implement the same behavior like DuckDB

I could directly check the returnType here do match the behavior of DuckDB

and make datafusion support query like

Don't know if it could meet the issue's need

[jayzhan211]

@Lordworms
I think arguments validity check are more like signature instead of one that need to be computed by the user in return_type.

[Lordworms]

@Lordworms I think arguments validity check are more like signature instead of one that need to be computed by the

Yes, I understand, but if we need to validate argument in signature a suitable way to do it is more likely what you did in #8594

user in return_type.

I'll try to see if I can come up with a better way, thanks for your advice!

[jayzhan211]

After #10439, I believe array_concat should use user-defined signature given it's speciality, therefore we can check the empty array in coerce_types for array_concat and return exec_error instead of panicking

cc @Lordworms

[jayzhan211]

I have a PR that updates the array_concat signature, maybe I can resolve the panic issue together.

[jayzhan211]

Actually, I'm thinking about whether we should change the behavior of array_concat similar to postgres and duckdb.
It is one of the earliest array functions that we don't follow what postgres and duckdb's behavior 🤔

[jonahgao]

Fixed by #10790