Add support for joins #175
Conversation
|
I am not opposed to supporting joins, but I think doing is this way would be problematic for a number of reasons:
The primary reason Spot doesn't support them right now is that in general, I feel like if you are building complex queries, an ORM is not the best tool for the job, and will most likely result in an unoptimized and inefficient query. This Tweet is a prime example of what I am talking about here (happened TODAY, in fact.) https://twitter.com/naderman/status/740901123666644992 |
|
I believe that this library has a lot of potential and missing the support for join, is one of the key feature missing( as discussed in other pull requests/issues). |
|
I agree that a good ORM should support joins, and would like to see them in Spot as an option for those who think differently than I do. I will help you work through it and support you all I can. I am looking forward to seeing what you come up with. 👍 |
this is reducing the complexity of the implementation and we can use the data already stored in Locator
sorinstanila
force-pushed
the
master
branch
2 times, most recently
from
9ddd785
to
74e9235
Compare
|
I am also want to add eager loading of entities/mappers which are selected with the join. Right now is only populated the mapper which the join is created. |
|
Don't worry about the eager loading of relations for now - that is enough work for a separate PR after JOIN support is in (if it is even needed at all). |
|
+1! :) |
|
Sorry all for the long time awaiting this. |
added support for custom conditions as array added missing code after merge from upstream
|
+1 |
| @@ -17,6 +17,10 @@ class RegExp | |||
| */ | |||
| public function __invoke(QueryBuilder $builder, $column, $value) | |||
| { | |||
| if ($value instanceof \Closure) { | |||
There was a problem hiding this comment.
Should this be done at a higher level since all these are the same? i.e. do this before passing in the $value to the type classes.
| $this->assertInstanceOf('\Spot\Entity\Manager', $manager); | ||
| $this->assertInstanceOf('\Spot\Entity\Manager', $managerCurrent); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't see any tests for the actual JOIN methods. Those need to be added to merge a feature that adds support for JOINs.
Do you think we need more to have support for joins?