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

[NFR] Binds on joins #27

Closed
brandonlamb opened this issue Jul 7, 2014 · 5 comments
Closed

[NFR] Binds on joins #27

brandonlamb opened this issue Jul 7, 2014 · 5 comments

Comments

@brandonlamb
Copy link

It's a recommended practice to move predicates to your joins so that your where clause is applied against a smaller subset. In the second example, the sql engine has to find all rows where id is less than the value, and then filter out type. The first example may do filtering first and then the id search is applied against a smaller subset.

Obviously the example "depends" and I may not have given a super great one below, but definitely a good feature request.

Example with:

SELECT id, name
FROM table1 AS t1
INNER JOIN table2 AS t2 ON (t2.table1_id = t1.id AND t2.type = ?)
WHERE t1.id < ?

Example without:

SELECT id, name
FROM table1 AS t1
INNER JOIN table2 AS t2 ON (t2.table1_id = t1.id)
WHERE t1.id < ? AND t2.type = ?

@pmjones
Copy link
Member

pmjones commented Jul 28, 2014

So we're thinking of changing the join() et al signatures to add a $bind argument at the end like with where() and having() ?

@brandonlamb
Copy link
Author

That's essentially what I ended up doing, and separated it into it's own query->join array (from is not join). Seeing as how we had the same thought, probably a decent approach haha.

$query->innerJoin('child2 c2', 'ON (c2.id = c1.child2_id AND c2.name = ?)', [$name]);

@pmjones
Copy link
Member

pmjones commented Jul 28, 2014

I get you. I was going to say "you can do it with named placeholders" but on review it looks easy enough to add to the join() signature.

@brandonlamb
Copy link
Author

One thing I like about the Spot DM is it automatically surrounds the join clause in ( ), which can help avoid some nasty bugs. I've found myself having to remember to type them out using aura. Just now above I had to edit because I forgot the ending parens.

On the flip side, Spot assumes the joins are using ON, so I like aura's freedom.

@pmjones
Copy link
Member

pmjones commented Mar 4, 2015

@brandonlamb see PR #50 for an implementation of this request and let me know if it satisfies.

@pmjones pmjones closed this as completed in 3019dcf Mar 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants