Fix concat() to support array inputs#19632
Conversation
|
Thanks for the review. Happy to make any changes if needed. |
|
Thanks @JuieeAJaviya -- there appears to be several test failures. Can you please try and fix them? |
|
fyi theres an existing PR for the issue (which doesn't seem to be linked here?) |
|
Thanks for the feedback! |
|
Thanks for the feedback! I’ve addressed the CI failures and updated the tests. |
|
@JuieeAJaviya Are you verifying this locally? That it builds and |
|
Thanks for checking and for pointing this out. |
|
Whilst we welcome contributions, we do expect at the very minimum they do compile and the basic test suite ( |
|
Thanks for the clarification and feedback. |
Rationale for this change
The
concat()scalar function currently treats array inputs as theirstring display representations, producing incorrect results such as
[1,2,3][4,5].DataFusion already provides correct array concatenation semantics via
array_concat(). Aligningconcat()behavior for array inputs withexisting SQL expectations improves correctness and consistency while
avoiding duplicated logic.
What changes are included in this PR?
concat()are arraysarray_concat()implementationconcat(array, array)behaves the same asarray_concat(array, array)Are these changes tested?
Yes.
A new unit test has been added to validate correct array concatenation
and to prevent regression. Existing tests continue to pass.
Are there any user-facing changes?
Yes.
concat()now returns correctly merged arrays when all inputs are arrays,matching the behavior of
array_concat(). Existing string behavior isunchanged.