Skip to content

Conversation

@jayzhan211
Copy link
Contributor

@jayzhan211 jayzhan211 commented Jul 7, 2023

Which issue does this PR close?

Ref #6804

Rationale for this change

Array concat with columns! Behavior is similar to Postgres.

Concat Operator a || b is not included in this PR!

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added physical-expr Changes to the physical-expr crates core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Jul 7, 2023
# [[5, 6], [7, 8], [5, 6], [7, 8]] [7.7, 8.8, 9.9, 7.7, 8.8, 9.9] [d, , l, o, r, d, , l, o, r]
# [[7, ], [9, 10], [7, ], [9, 10]] [10.1, , 12.2, 10.1, , 12.2] [s, i, t, s, i, t]
# NULL [13.3, 14.4, 15.5, 13.3, 14.4, 15.5] [a, m, e, t, a, m, e, t]
# [[11, 12], [13, 14], [11, 12], [13, 14]] 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.

Not sure do we need an additional "" for commas.

@jayzhan211 jayzhan211 marked this pull request as ready for review July 11, 2023 06:15
@jayzhan211
Copy link
Contributor Author

Not all kinds of tests are passed but I think they can be solved in another PR. I think we can review this PR first.

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
@jayzhan211
Copy link
Contributor Author

@izveigor @alamb Ready for review! Thanks!


let builder = mutable.into_builder();

let list = builder
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to consider the method try_new? (https://docs.rs/arrow/latest/arrow/array/struct.GenericListArray.html#method.try_new) 🤔

Copy link
Contributor Author

@jayzhan211 jayzhan211 Jul 13, 2023

Choose a reason for hiding this comment

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

I prefer to construct with ListArray::new instead of MutableArrayData too, but I fail on this.

Copy link
Contributor

@izveigor izveigor left a comment

Choose a reason for hiding this comment

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

Thank you, @jayzhan211! LGTM!

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.

Thank you @izveigor and @jayzhan211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate 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.

3 participants