Skip to content

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 Changes to the physical-expr crates 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. 🚀

----
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 Changes to the physical-expr crates 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