Skip to content

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

Merged
Dandandan merged 1 commit intoapache:masterfrom
jimexist:fix-226
Jul 22, 2021
Merged

fix 226, make concat, concat_ws, and random work with Python crate#761
Dandandan merged 1 commit intoapache:masterfrom
jimexist:fix-226

Conversation

@jimexist
Copy link
Member

@jimexist jimexist commented Jul 21, 2021

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?

@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

jimexist commented Jul 21, 2021

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

+1 more reviewer

@jorgecarleitao jorgecarleitao jorgecarleitao approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

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