Add support for joins - Githubissues

spotorm / spot2

Spot v2.x DataMapper built on top of Doctrine's Database Abstraction Layer

http://phpdatamapper.com

BSD 3-Clause "New" or "Revised" License

601 stars 101 forks source link

Add support for joins #175

Open sorinstanila opened 8 years ago

sorinstanila commented 8 years ago

Do you think we need more to have support for joins?

vlucas commented 8 years ago

I am not opposed to supporting joins, but I think doing is this way would be problematic for a number of reasons:

Spot uses custom syntax for WHERE conditions that is simpler than what DBAL wants. Any official JOIN support would have to use the known Spot syntax and translate it to the JOIN.
Fields in Spot can have aliases that need to be accounted for in JOIN conditions and columns
It probably should be possible to JOIN on a Spot Entity class name in addition to a raw table name
... and many other concerns like that.

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

sorinstanila commented 8 years ago

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). Yes, of course, we can find bad example for anything related to database loading, but if do not try to improve and learn from, it would not help :) I will look on the things you mentioned and update the requests. Any other feedback related to the implementation is welcomed and appreciated :)

vlucas commented 8 years ago

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. 👍

sorinstanila commented 8 years ago

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. It still need to add some tests.

vlucas commented 8 years ago

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).

mdular commented 8 years ago

+1! :)

sorinstanila commented 8 years ago

Sorry all for the long time awaiting this. I broke my arm and I am still in recovery period. In 3 weeks I hope to get back :)

luklub commented 8 years ago

+1 The lack of this feature is very noticeable.