Relation#from to accept other Relation objects

[RStankov]

I had to use subqueries in my project, several times. And I used this pattern:

Record.from("(#{sub_query.to_sql})")
Record.from("(#{sub_query.to_sql}) a")

Which doesn't feel right. I much prefer just to pass ActiveRecord::Relation objects:

Record.from("(#{sub_query.to_sql})")    -> Record.from(sub_query)
Record.from("(#{sub_query.to_sql}) a")  -> Record.from(:a => sub_query)

So this pull request add this functionality.

[larzconwell]

👍

[jeremyf]

The tests all pass for this pull request.

[rafaelfranca]

cc/ @jonleighton @tenderlove

[jonleighton]

The seems good in principle. Some comments:

• Needs documentation
• I'm not keen on the Record.from(:a => sub_query) API. Would prefer Record.from(sub_query, :a) so the optional second parameter is an alias.
• I don't think the build_from stuff should be done when #from is called - it should happen when we build the arel.
• Needs a rebase

[jonleighton]

Not sure about the default alias here. Presumably we need to have one? How about just calling it "subquery"?

[RStankov]

Done

[RStankov]

@jonleighton

I had:

• Added documentation
• Made subquery alias name as second optional argument
• Made subquery default alias to be "subquery"
• rebased

The only thing left todo, is to postpone build_from to be called in build_arel. Unfortunately I'm not very sure which is the best way to do this. But I will probably figure this out tomorrow.

[steveklabnik]

Needs another rebase, apparently.

[frodsan]

Remove additional parentheses:

# => SELECT a.title FROM (SELECT * FROM topics) WHERE approved = 't') a

to:

# => SELECT a.title FROM (SELECT * FROM topics WHERE approved = 't') a

[tenderlove]

One more rebase please.

I like this feature. I'll apply if @jonleighton is happy too!

[jonleighton]

I like it too. But I think we need to get the build_from logic into build_arel before applying :)

[RStankov]

New rebase :)

I have moved build_from into build_arel ( I actually liked more, the version where build_from was used in from!)

I can do a little bit more cleanup there. Just want to check if you approve this approach.

[jonleighton]

Looks good to me. The reason I wanted to do it in build_arel is because it makes the overhead of converting the from relation lazy. So if we don't need it in the end, then we won't have to do that work.

[jonleighton]

When you're done, please squash everything into a single commit and let us know so we can merge. Thanks.

[RStankov]

I'm done