Skip to content

[5.6] Add helpers for subquery join clauses#23818

Merged
taylorotwell merged 2 commits intolaravel:5.6from
staudenmeir:5.6
Apr 17, 2018
Merged

[5.6] Add helpers for subquery join clauses#23818
taylorotwell merged 2 commits intolaravel:5.6from
staudenmeir:5.6

Conversation

@staudenmeir
Copy link
Copy Markdown
Contributor

@staudenmeir staudenmeir commented Apr 5, 2018

This PR adds joinSub(), leftJoinSub() and rightJoinSub() to the query builder. The goal is to make subquery joins easier.

At the moment subquery joins are possible using raw SQL:

$query = DB::table('subtable');
$sql = '(' . $query->toSql() . ') as "sub"';
DB::table('table')->join(DB::raw($sql), ...);

That's ok for simple subqueries but gets a bit tedious if the subquery has bindings:

$query = DB::table('subtable')->where('foo', 'bar');
$sql = '(' . $query->toSql() . ') as "sub"';
DB::table('table')->addBinding($query->getBindings(), 'join')->join(DB::raw($sql), ...);

With the new helpers users can create subquery joins using raw SQL, a closure or a query builder:

DB::table('table')->joinSub('select * from "subtable"', 'sub', ...);
DB::table('table')->joinSub(function ($q) { $q->from('subtable'); }, 'sub', ...);
DB::table('table')->joinSub(DB::table('subtable')->where('foo', 'bar'), 'sub', ...);

@sisve
Copy link
Copy Markdown
Contributor

sisve commented Apr 7, 2018

There are no tests that verify that it works to joinSub several times in one query. Can you add that? Something like DB::table('table')->joinSub('...', 'sub1')->joinSub('...', 'sub2')?

@staudenmeir
Copy link
Copy Markdown
Contributor Author

@sisve I added a test with multiple subquery joins.

@BrandonShar
Copy link
Copy Markdown
Contributor

How do you feel about using joinSubQuery instead of joinSub? Feels like it's a lot easier to read/understand at a glance and is only 5 more characters. I think it's good to avoid abbreviations in code unless they have some clear benefit.

@staudenmeir
Copy link
Copy Markdown
Contributor Author

@BrandonShar I chose the name to match the existing selectSub() and fromSub().

@BrandonShar
Copy link
Copy Markdown
Contributor

ah, that makes sense; wasn't familiar with those.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants