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: new concatenation operator for working with arrays #6615

Merged
merged 5 commits into from
Jun 15, 2023
Merged

feat: new concatenation operator for working with arrays #6615

merged 5 commits into from
Jun 15, 2023

Conversation

izveigor
Copy link
Contributor

@izveigor izveigor commented Jun 9, 2023

Which issue does this PR close?

Closes #6603

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Yes

Are there any user-facing changes?

Yes

@github-actions github-actions bot added core Core DataFusion crate logical-expr Logical plan and expressions physical-expr Physical Expressions sqllogictest SQL Logic Tests (.slt) labels Jun 9, 2023
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

I am a huge fan of adding functionality with net negative code! Nice work @izveigor

Screenshot 2023-06-11 at 10 38 31 AM

I had some testing suggestions, but otherwise this looks good to me. 🚀

@@ -204,3 +204,21 @@ query II rowsort
select array_ndims(array_fill(1, [1, 2, 3])), array_ndims([[[[[[[[[[[[[[[[[[[[[1]]]]]]]]]]]]]]]]]]]]]);
----
3 21

# array concatenate operator #1 (like array_concat scalar function)
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend adding a test for `null handling in these arrays

like

select make_array(1, 2, 3) || make_array(4, null, 6);

and

select make_array(1, 2, 3) || null;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this feature will only be available after issue #6556 is resolved.

@izveigor
Copy link
Contributor Author

@alamb, what do you think about this PR?

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

I think this looks great -- thank you @izveigor

I took the liberty of merging this PR up from main to resolve a conflict and I'll plan to merge the PR when tests pass

@alamb
Copy link
Contributor

alamb commented Jun 14, 2023

https://github.com/apache/arrow-datafusion/actions/runs/5271901967/jobs/9533617876?pr=6615

🤔 that is not good:

= note: /usr/bin/ld: final link failed: No space left on device

I restarted the test

@alamb alamb merged commit 6872535 into apache:main Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate logical-expr Logical plan and expressions physical-expr Physical Expressions sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New concatenation operator for working with arrays
2 participants