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

Add database specific string concatenation #410

Merged
merged 1 commit into from
Jan 6, 2016
Merged

Add database specific string concatenation #410

merged 1 commit into from
Jan 6, 2016

Conversation

kbrock
Copy link

@kbrock kbrock commented Jan 1, 2016

Hi All

String concatenation is database specific.

Postgres (and ANSI SQL) supports column1 || column2, but databases like mysql use concat(column1, column2).

This PR introduces string concatenation to Arel to abstract this nuance. e.g.: table[:name].concat(other).
Before this, developers needed to build database specific sql on their own.

If Arel is not the right place for this abstraction, do you have any suggestions on how to abstract this in my own app?

Thanks,
Keenan

/cc @Fryguy @matthewd (and of course @tenderlove ) This is for ancestry, but I feel we want something similar in our spot as well.

@rails-bot
Copy link

r? @rafaelfranca

(@rails-bot has picked a reviewer for you, use r? to override)

@kbrock
Copy link
Author

kbrock commented Jan 1, 2016

It looks like #173 was closed, but not exactly sure why. Is it because this is not the right layer for this abstraction, or because that PR not support mysql?

Thanks

@kbrock
Copy link
Author

kbrock commented Jan 6, 2016

@rafaelfranca Does this look like something that could go into Arel?

thanks

@rafaelfranca
Copy link
Member

Yes. It makes sense. Thank you for the pull request.

rafaelfranca added a commit that referenced this pull request Jan 6, 2016
Add database specific string concatenation
@rafaelfranca rafaelfranca merged commit ffc4b8e into rails:master Jan 6, 2016
@kbrock kbrock deleted the concat branch January 6, 2016 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants