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.

Sign up for GitHub

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

Jump to bottom

fix 226, make concat, concat_ws, and random work with Python crate #761

Merged
merged 1 commit into from
Jul 22, 2021
Merged

fix 226, make concat, concat_ws, and random work with Python crate #761

merged 1 commit into from
Jul 22, 2021

Conversation

jimexist
Copy link
Member

Which issue does this PR close?

Closes #226

Rationale for this change

  • concat and concat_ws no longer works

What changes are included in this PR?

  • fix concat
  • fix concat_ws
  • fix random
  • add unit tests

Are there any user-facing changes?

@github-actions github-actions bot added the datafusion Changes in the datafusion crate label Jul 21, 2021
@jimexist
fix 226
1721dae 
- fix concat
- fix concat_ws
- fix random
- add unit tests
@jimexist jimexist changed the title fix 226 fix 226, make concat, concat_ws, and random work with Python create Jul 21, 2021
@jimexist jimexist changed the title fix 226, make concat, concat_ws, and random work with Python create fix 226, make concat, concat_ws, and random work with Python crate Jul 21, 2021
jorgecarleitao
jorgecarleitao approved these changes Jul 21, 2021
View reviewed changes
Copy link
Member

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot, @jimexist !

@alamb alamb mentioned this pull request Jul 21, 2021
Merged
alamb
alamb approved these changes Jul 21, 2021
View reviewed changes
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.

Thanks @jimexist ! Looks good to me -- we often add a pub use in the logical_plan/expr.rs for functions such as concat, concat_ws, and random which might be useful too

https://github.com/apache/arrow-datafusion/blob/master/datafusion/src/logical_plan/mod.rs#L38-L48

@jimexist
Copy link
Member Author

Thanks @jimexist ! Looks good to me -- we often add a pub use in the logical_plan/expr.rs for functions such as concat, concat_ws, and random which might be useful too

https://github.com/apache/arrow-datafusion/blob/master/datafusion/src/logical_plan/mod.rs#L38-L48

i think it's already in that pub use list

@Dandandan Dandandan merged commit 4d61196 into apache:master Jul 22, 2021
@jimexist jimexist deleted the fix-226 branch July 22, 2021 06:54
@alamb
Copy link
Contributor

alamb commented Jul 22, 2021

i think it's already in that pub use list

Good call -- sorry I missed that

@houqp houqp added the python label Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dandandan Dandandan Dandandan approved these changes

@alamb alamb alamb approved these changes

@jorgecarleitao jorgecarleitao jorgecarleitao approved these changes

Assignees
No one assigned
Labels
datafusion Changes in the datafusion crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Concat from Dataframe API no longer accepts multiple expressions
5 participants
@jimexist @alamb @Dandandan @jorgecarleitao @houqp